Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
📝 WalkthroughWalkthroughThis pull request refactors the API handlers to separate OpenAPI route descriptors from their implementations. Route metadata (schemas, paths, response definitions) are extracted to dedicated Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Refactors ENSApi’s OpenAPI route metadata so it can be shared between the runtime server and spec-generation contexts without pulling in config/env-dependent modules.
Changes:
- Extracted
describeRoutemetadata into dedicated*.routes.tsmodules per handler. - Centralized OpenAPI
documentation(info/servers/tags) intosrc/openapi.tsand reused it in the runtime/openapi.jsonendpoint. - Added a lightweight
createRoutesForSpec()route composer (src/routes.ts) that mounts only metadata + stub handlers for spec generation.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/ensapi/src/routes.ts | Adds spec-only Hono app composition using stub handlers and imported route metadata. |
| apps/ensapi/src/openapi.ts | Centralizes OpenAPI documentation metadata (info/servers/tags). |
| apps/ensapi/src/index.ts | Uses openapiDocumentation for /openapi.json, overriding version and adding localhost server. |
| apps/ensapi/src/handlers/resolution-api.ts | Replaces inline describeRoute objects with imports from resolution-api.routes.ts. |
| apps/ensapi/src/handlers/resolution-api.routes.ts | New module containing OpenAPI metadata + response schemas for resolution endpoints. |
| apps/ensapi/src/handlers/registrar-actions-api.ts | Replaces inline describeRoute objects with imports from registrar-actions-api.routes.ts. |
| apps/ensapi/src/handlers/registrar-actions-api.routes.ts | New module containing OpenAPI metadata for registrar-actions endpoints. |
| apps/ensapi/src/handlers/name-tokens-api.ts | Replaces inline describeRoute objects with import from name-tokens-api.routes.ts. |
| apps/ensapi/src/handlers/name-tokens-api.routes.ts | New module containing OpenAPI metadata + response schemas for name-tokens endpoint. |
| apps/ensapi/src/handlers/ensnode-api.ts | Replaces inline describeRoute objects with imports from ensnode-api.routes.ts. |
| apps/ensapi/src/handlers/ensnode-api.routes.ts | New module containing OpenAPI metadata + response schemas for config/indexing-status endpoints. |
| apps/ensapi/src/handlers/ensanalytics-api.ts | Replaces inline describeRoute objects with imports from ensanalytics-api.routes.ts. |
| apps/ensapi/src/handlers/ensanalytics-api.routes.ts | New module containing OpenAPI metadata for ENSAnalytics v0 endpoints. |
| apps/ensapi/src/handlers/ensanalytics-api-v1.ts | Replaces inline describeRoute objects with imports from ensanalytics-api-v1.routes.ts. |
| apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts | New module containing OpenAPI metadata for ENSAnalytics v1 endpoints. |
| apps/ensapi/src/handlers/amirealtime-api.ts | Replaces inline describeRoute object with import from amirealtime-api.routes.ts. |
| apps/ensapi/src/handlers/amirealtime-api.routes.ts | New module containing OpenAPI metadata for /amirealtime. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts`:
- Around line 4-63: The route descriptors getReferralLeaderboardV1Route,
getReferrerDetailV1Route, and getEditionConfigSetRoute currently only list
response descriptions—update each responses object to include content entries
that attach JSON schemas via validationResolver (same pattern used in
name-tokens-api.routes.ts) so the OpenAPI spec exposes response body schemas;
include the appropriate status codes (e.g., 200 success schemas and
400/404/500/503 error schemas where applicable) and reference the existing
response schema validators for leaderboard pages, referrer detail, edition
config set, and shared error shapes.
In `@apps/ensapi/src/handlers/ensnode-api.routes.ts`:
- Line 9: Replace the explicit type annotation on your route exports with a
"satisfies DescribeRouteOptions" clause to preserve literal type inference
(e.g., tag strings); specifically update getConfigRoute (and the other route
export at the second occurrence) to use "satisfies DescribeRouteOptions" instead
of ": DescribeRouteOptions" so the routes match the style used in
amirealtime-api.routes.ts, registrar-actions-api.routes.ts, and
ensanalytics-api-v1.routes.ts.
In `@apps/ensapi/src/handlers/resolution-api.routes.ts`:
- Around line 10-56: Add documented error responses to each route metadata
(getResolveRecordsRoute, getResolvePrimaryNameRoute,
getResolvePrimaryNamesRoute): include common error status entries such as 400
(invalid input), 404 (not found), and 500 (internal error) under the responses
object and reference the appropriate error response schema (e.g., a shared
validationErrorSchema or genericErrorSchema via validationResolver) so the
OpenAPI spec describes possible failures as well as the 200 success case.
In `@apps/ensapi/src/handlers/resolution-api.ts`:
- Around line 21-25: The route descriptors returned by
getResolvePrimaryNameRoute, getResolvePrimaryNamesRoute, and
getResolveRecordsRoute only document a 200 response; update each descriptor to
include standard error responses (e.g., 400, 404, 500, 503) matching the style
used in name-tokens-api.routes.ts so the OpenAPI spec covers middleware
invariant failures and thrown errors; modify the response objects in those
functions to add descriptive schemas/messages for each error status and ensure
any shared error schema is reused if present.
In `@apps/ensapi/src/routes.ts`:
- Line 40: The stub handler currently uses an untyped parameter (const stub =
(c: any) => c.text("")); replace the any with a minimal Hono type: import {
Context } from "hono" and change the signature to use Context (e.g., const stub
= (c: Context) => c.text("") or declare the return as Response/Promise<Response>
if async); update the parameter name if desired and ensure the import for
Context is added so the handler is properly typed.
- Around line 42-85: The route strings in createRoutesForSpec are hard-coded and
can drift from the handler definitions; refactor by exporting path constants
from each handler module (e.g., export const CONFIG_ROUTE = "/api/config" in the
module that defines getConfigRoute, REGISTRAR_ACTIONS_ROUTE for
getRegistrarActionsRoute, AM_I_REALTIME_ROUTE for getAmIRealtimeRoute, etc.) and
import those constants into routes.ts to use in createRoutesForSpec instead of
literal strings, ensuring each handler (getConfigRoute, getIndexingStatusRoute,
getNameTokensRoute, getRegistrarActionsRoute,
getRegistrarActionsByParentNodeRoute, getResolveRecordsRoute,
getResolvePrimaryNameRoute, getResolvePrimaryNamesRoute,
getReferrerLeaderboardRoute, getReferrerDetailRoute,
getReferralLeaderboardV1Route, getReferrerDetailV1Route,
getEditionConfigSetRoute, getAmIRealtimeRoute) exports its path constant so both
the handler and the router share the single source of truth.
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Only 200 responses are documented — error responses are missing from the spec.
All three resolution routes only define a 200 response. If these endpoints can return error status codes (e.g., 400 for invalid input, 404, 500), they should be documented here for a complete OpenAPI spec. This may be pre-existing, but since you're centralizing route metadata, now is a good opportunity to add them.
🤖 Prompt for AI Agents
In `@apps/ensapi/src/handlers/resolution-api.routes.ts` around lines 10 - 56, Add
documented error responses to each route metadata (getResolveRecordsRoute,
getResolvePrimaryNameRoute, getResolvePrimaryNamesRoute): include common error
status entries such as 400 (invalid input), 404 (not found), and 500 (internal
error) under the responses object and reference the appropriate error response
schema (e.g., a shared validationErrorSchema or genericErrorSchema via
validationResolver) so the OpenAPI spec describes possible failures as well as
the 200 success case.
lightwalker-eth
left a comment
There was a problem hiding this comment.
@notrab Sharing some preliminary feedback 😄
There was a problem hiding this comment.
Maybe instead of calling each of these files a ".routes.ts" file we call it something like ".meta.ts" since I understand each of these is metadata about the route, not the route implementation itself.
There was a problem hiding this comment.
Perhaps we should move things like referrerLeaderboardPageQuerySchema into these new separate metadata files? Appreciate your advice.
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/package.json`:
- Line 31: Update the pinned dependency "@hono/zod-openapi" in apps/ensapi's
package.json from "1.0.0-beta.1" to a stable release (at minimum "1.0.0" or
preferably "1.2.1"); then run your package manager to regenerate the lockfile
(npm install / yarn install / pnpm install) and run the test/build (or linter)
to catch any breaking changes or type adjustments in code paths that import or
reference "@hono/zod-openapi".
- Around line 20-21: The package.json "typecheck" script references the
unavailable tool "tsgo" (scripts: "typecheck") which causes CI/local failures;
either add "tsgo" as a devDependency across the workspace (pin a compatible
version) and update lockfiles, or change the "typecheck" script to use the
standard TypeScript compiler ("tsc") instead (matching how "generate:openapi"
uses "tsx"); update package.json's "typecheck" entry accordingly and run install
to ensure the tool is present.
In `@apps/ensapi/scripts/generate-openapi.ts`:
- Line 17: The generated OpenAPI spec always contains version "0.0.0" because
openapiDocumentation.info.version is never updated before calling
app.getOpenAPI31Document; update generate-openapi.ts to read the package.json
version (e.g., require or fs.readFileSync + JSON.parse on package.json) and
assign that value to openapiDocumentation.info.version before invoking
app.getOpenAPI31Document(openapiDocumentation) so the produced spec contains the
real package version.
In `@apps/ensapi/src/handlers/amirealtime-api.routes.ts`:
- Around line 31-40: The OpenAPI responses in amirealtime-api.routes.ts lack
response bodies: update the responses object for status 200 to include content:
application/json with a schema describing the success payload (properties:
maxWorstCaseDistance, slowestChainIndexingCursor, worstCaseDistance with
appropriate types) and for 503 add content: application/json with a schema
matching the errorResponse shape (message: string); ensure these schemas align
with the handler amirealtime-api.ts return values so the spec accurately
documents the JSON payloads.
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts`:
- Line 10: The project imports Zod v4 (see the import { z } from "zod/v4") but
the pinned dependency `@hono/zod-openapi` is an old beta that only supports Zod
v3; update your package dependency to `@hono/zod-openapi`@^1.2.0 (or later) in
package.json/pnpm lockfile and run install so the library matching Zod v4 is
used, then run type checks and tests to ensure no breaking import or type
mismatches in places using z and any zod-openapi utilities.
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.test.ts`:
- Line 94: The test declares the test client as "const client: any =
testClient(app)", which silences type checking for route methods and responses;
update this by keeping the existing runtime behavior but adding a clear TODO
explaining the temporary use of "any" and linking it to the current beta of
`@hono/zod-openapi` so it gets revisited when app.openapi()/the lib
stabilizes—specifically annotate the variable "client" (where testClient(app) is
called) with a comment like "// TODO: temporary any due to `@hono/zod-openapi`
beta; replace with proper typed client when app.openapi() exposes stable types"
so future maintainers know to remove the any when the library matures.
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.ts`:
- Around line 204-219: The extra runtime/type-guard filter creating
validEditionLeaderboards is redundant because the earlier
uncachedEditions.length > 0 guard guarantees no Error entries; remove the filter
and use editionLeaderboards directly (or assert its type) when building
editionsData with getReferrerEditionMetrics, e.g. treat editionLeaderboards as
Array<{ editionSlug: ReferralProgramEditionSlug; leaderboard:
ReferrerLeaderboard }> and produce editionsData as ReferrerMetricsEditionsData
to simplify the code and eliminate validEditionLeaderboards.
- Around line 107-121: The catch block in the
/v1/ensanalytics/referral-leaderboard handler leaks raw exception messages to
clients by inserting error.message into the serialized
ReferrerLeaderboardPageResponse; update the catch blocks (the one using
logger.error and returning serializeReferrerLeaderboardPageResponse with
ReferrerLeaderboardPageResponseCodes.Error) to log the full error server-side
via logger.error({ error }, "...") but instead return a generic client-facing
errorMessage (e.g., "An unexpected error occurred") and a fixed error field
(e.g., "Internal server error"); apply the same change to the other two handlers
that build responses using the same pattern so only the server log contains the
raw error and responses expose no sensitive internals.
- Around line 40-46: The invariant checks that throw raw Errors (seen around
getReferralLeaderboardV1Route and the other route handlers checking
referralLeaderboardEditionsCaches) must not throw outside the try/catch; instead
move each check (the conditional that throws `Invariant(ensanalytics-api-v1):
referralLeaderboardEditionsCachesMiddleware required`) into the surrounding try
block of the handler or replace the throw with an immediate structured JSON
error response (e.g., return c.json({ error: "Invariant...", detail: ... },
500)); update the checks in getReferralLeaderboardV1Route and the other handlers
(the blocks that reference referralLeaderboardEditionsCachesMiddleware) so they
either run inside try or return a c.json 500 to ensure the catch block / API
contract is preserved.
In `@apps/ensapi/src/handlers/ensanalytics-api.routes.ts`:
- Around line 29-31: The referrerAddressSchema definition is duplicated; extract
it into a shared schemas module (e.g., export const referrerAddressSchema =
z.object({ referrer: makeLowercaseAddressSchema("Referrer
address").describe("Referrer Ethereum address") });) and replace the local
declarations in ensanalytics-api.routes.ts and ensanalytics-api-v1.routes.ts
with imports from that module; update imports where the symbol
referrerAddressSchema and the helper makeLowercaseAddressSchema are referenced
so both route files import the canonical referrerAddressSchema from the shared
module.
In `@apps/ensapi/src/handlers/ensnode-api.ts`:
- Around line 54-60: The route mounting uses fragile substring removal via
nameTokensBasePath.replace(basePath, ""),
registrarActionsBasePath.replace(basePath, ""), and
resolutionBasePath.replace(basePath, ""), which can silently produce incorrect
paths; change these to explicitly strip the leading basePath using slice by
verifying the target string startsWith(basePath) and then using
.slice(basePath.length) before passing to app.route so you mount the correct
sub-paths (use nameTokensBasePath, registrarActionsBasePath, resolutionBasePath,
basePath, and app.route to locate the edits).
In `@apps/ensapi/src/handlers/name-tokens-api.routes.ts`:
- Around line 38-80: Extract the repeated schema creation into a single constant
and reuse it in the responses: create a const (e.g., nameTokensResponseSchema =
makeNameTokensResponseSchema("Name Tokens Response", true)) near the top of the
handler file, then replace the three calls to makeNameTokensResponseSchema("Name
Tokens Response", true) in the responses object (for the 200, 404, and 503
entries) with that constant; ensure the constant is in scope for the responses
object and that ErrorResponseSchema usages remain unchanged.
In `@apps/ensapi/src/handlers/name-tokens-api.ts`:
- Around line 106-114: The error message template literal passed into
makeNameTokensNotIndexedResponse is missing a closing single quote around the
interpolated name; update the string passed in that call (the argument to
makeNameTokensNotIndexedResponse used inside serializeNameTokensResponse /
c.json) so it reads `...for the requested name: '${name}'` (i.e., close the
single quote before the final backtick) to fix the unterminated string.
- Line 43: The handler passed to app.openapi for getNameTokensRoute currently
declares an explicit return type of Promise<any>, which disables Hono's
type-checking; remove the explicit ": Promise<any>" so TypeScript infers the
correct return type from the route/context (or replace it with
"Promise<Response>" if you truly need a concrete type) in the async function
passed to app.openapi(getNameTokensRoute, ...); update the async (c):
Promise<any => { ... } signature to either async (c) => { ... } or async (c):
Promise<Response> => { ... } to restore type-safe response validation.
In `@apps/ensapi/src/handlers/registrar-actions-api.routes.ts`:
- Around line 28-40: The schema uses numeric defaults on a string-typed
intermediate schema (params.queryParam.pipe(z.coerce.number())), which is
confusing; change the defaults to string values so they match the intermediate
type: update the `page` default from 1 to "1" and the `recordsPerPage` default
from RECORDS_PER_PAGE_DEFAULT (number) to a string form (e.g.,
String(RECORDS_PER_PAGE_DEFAULT)), keeping the subsequent
.pipe(z.coerce.number()) and .pipe(makePositiveIntegerSchema("page") /
makePositiveIntegerSchema("recordsPerPage").max(RECORDS_PER_PAGE_MAX)) intact so
runtime coercion and validation are unchanged.
In `@apps/ensapi/src/handlers/resolution-api.routes.ts`:
- Around line 88-97: The response description in getResolvePrimaryNamesRoute is
a copy-paste that says "Successfully resolved records"; update the 200 response
description to reference primary names (e.g. "Successfully resolved primary
names") in the route definition that uses
makeResolvePrimaryNamesResponseSchema() so the description matches the handler's
purpose.
In `@apps/ensapi/src/index.ts`:
- Line 70: The route mounting for subgraphApi is inconsistent:
app.route("/subgraph", subgraphApi) uses a hardcoded path while other routers
use imported basePath constants; either create a new module named
subgraph-api.routes.ts that exports a basePath (e.g., export const
subgraphBasePath = "/subgraph") alongside the router export and import that
constant into index.ts to replace the hardcoded string, or add a succinct inline
comment next to app.route("/subgraph", subgraphApi) explaining why this route is
intentionally not using the basePath pattern; update imports/usages of
subgraphApi in index.ts to use the exported basePath if you add the file.
In `@apps/ensapi/src/openapi.ts`:
- Line 4: The OpenAPI title string is redundant ("ENSApi APIs"); update the
title property in openapi.ts (the title symbol) to a non-redundant value such as
"ENSApi" or "ENSNode APIs" so it no longer reads "API APIs" (replace the
existing "ENSApi APIs" value with the chosen string).
In `@apps/ensapi/src/routes.ts`:
- Line 71: The route object is being cast with `as any` when calling
`app.openapi({ ...route, path: fullPath } as any, stub)` because
`RouteGroup.routes` was widened and lost the original route type; restore a
precise type for `RouteGroup.routes` (e.g., `ReadonlyArray<ReturnType<typeof
createRoute>>` or the exact route return type used by your router) so the
`route` variable carries full OpenAPI metadata, then remove the `as any` cast
and call `app.openapi({ ...route, path: fullPath }, stub)` directly; update any
related types/signatures (e.g., the `createRoute` factory and `RouteGroup`
definition) so TypeScript infers the proper shape without needing casts.
- Line 15: Upgrade the `@hono/zod-openapi` dependency from the beta to the stable
1.0.2 release in your package manifest (replace version "1.0.0-beta.1" with
"1.0.2"), reinstall dependencies (npm/yarn/pnpm install), then rebuild and run
tests to ensure the existing import OpenAPIHono in routes.ts remains compatible;
if any type or API changes surface, adjust usages of OpenAPIHono accordingly to
match the stable release's signatures.
---
Duplicate comments:
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts`:
- Around line 53-131: Each route's responses currently only include
descriptions; update getReferralLeaderboardV1Route, getReferrerDetailV1Route,
and getEditionConfigSetRoute to include a content entry for each relevant
response (e.g., 200, 400, 404, 500, 503) with "application/json" and the
appropriate schema reference (or inline schema) for the response body instead of
just a description; for example add responses: { 200: { description: "...",
content: { "application/json": { schema: { $ref:
"#/components/schemas/ReferrerLeaderboardPage" } } } } } and similarly
reference/define schemas for ReferrerDetailResponse, EditionConfigSetResponse
and ErrorResponse (or create those component schemas) so the OpenAPI spec
includes proper JSON body shapes for each status.
In `@apps/ensapi/src/handlers/ensanalytics-api.routes.ts`:
- Around line 33-72: The route response objects for getReferrerLeaderboardRoute
and getReferrerDetailRoute currently only include descriptions; add proper
response body schemas for the 200 success cases (e.g., a paginated leaderboard
item schema for getReferrerLeaderboardRoute and a referrer detail schema for
getReferrerDetailRoute) and standard error schema definitions for 500 and 503
responses (reuse your existing error/schema types if available). Update the
responses map for getReferrerLeaderboardRoute and getReferrerDetailRoute to
reference these JSON schemas (or TypeScript schema objects) so the API docs and
validation include the response shapes alongside the existing request schemas
(paginationQuerySchema, referrerAddressSchema).
In `@apps/ensapi/src/handlers/ensanalytics-api.test.ts`:
- Line 57: Tests instantiate a loosely-typed test client using "const client:
any = testClient(app)" which repeats across v1/v2 tests; consolidate this
workaround by creating a shared helper (e.g., a typedTestClient or testClientAny
wrapper) or add a central TODO comment referenced by the helper to track the
`any` usage; update usages in files like ensanalytics-api.test.ts to call that
shared helper (replace direct "const client: any = testClient(app)" with the
helper) so the pattern is centralized and easier to change later.
In `@apps/ensapi/src/handlers/ensanalytics-api.ts`:
- Around line 26-70: Move the invariant check for c.var.referrerLeaderboard
(used by getReferrerLeaderboardRoute) inside the existing try block so the
thrown invariant becomes catchable; do the same pattern used in the v1 handler.
In the catch for getReferrerLeaderboardRoute, stop returning error.message to
clients: log the full error object with logger.error({ error }, ...) and return
a generic errorMessage in the serialized response
(serializeReferrerLeaderboardPageResponse with
ReferrerLeaderboardPageResponseCodes.Error) so no internal error details leak to
the client.
- Around line 74-117: Move the invariant check that ensures
referrerLeaderboardMiddleware ran (the c.var.referrerLeaderboard undefined check
in the app.openapi handler) inside the try block so it is caught by the handler;
also avoid returning internal error messages to clients by not exposing
error.message directly—keep logger.error({ error }, "Error in
/ensanalytics/referrers/:referrer endpoint") for detailed internal logs but
return a generic errorMessage to the client (e.g., "An unexpected error occurred
while processing your request") in the catch block for the
serializeReferrerDetailResponse error path; references: app.openapi handler,
c.var.referrerLeaderboard, referrerLeaderboardMiddleware, getReferrerDetail, and
logger.error.
In `@apps/ensapi/src/handlers/resolution-api.ts`:
- Around line 47-141: Add explicit error responses to the OpenAPI descriptors
for getResolveRecordsRoute, getResolvePrimaryNameRoute, and
getResolvePrimaryNamesRoute: update each app.openapi(...) call to include the
common error response schemas (e.g., 400 Bad Request, 401 Unauthorized if auth
can fail, 404 Not Found, and 500 Internal Server Error) and reference the same
error response model used elsewhere in the project (so consumers see error shape
and status codes). Ensure the responses reference the correct operation names
resolveForward, resolveReverse, and resolvePrimaryNames so the route descriptors
document expected failures alongside the successful
ResolveRecordsResponse/ResolvePrimaryNameResponse/ResolvePrimaryNamesResponse
shapes.
In `@apps/ensapi/src/routes.ts`:
- Line 62: Replace the untyped stub handler by giving the parameter a minimal
interface instead of any: update the stub function (stub) to accept a context
typed with a text method (e.g., c: { text: (body?: string) => void }) and return
void, so callers and type-checker know the shape of the context without using
any.
| "typecheck": "tsgo --noEmit", | ||
| "generate:openapi": "tsx scripts/generate-openapi.ts" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if tsgo is referenced elsewhere in the repo (CI config, root package.json, etc.)
rg -n 'tsgo' --type-add 'config:*.json,*.yaml,*.yml,*.toml' -g '!node_modules'
Repository: namehash/ensnode
Length of output: 830
🏁 Script executed:
#!/bin/bash
# Also check for tsx references and verify both tools in package.json files
echo "=== Checking for tsx references ==="
rg -n 'tsx' --type-add 'config:*.json,*.yaml,*.yml,*.toml' -g '!node_modules'
Repository: namehash/ensnode
Length of output: 50374
🏁 Script executed:
#!/bin/bash
# Check the apps/ensapi/package.json for dependencies
echo "=== Checking apps/ensapi/package.json devDependencies ==="
cat apps/ensapi/package.json | head -50
Repository: namehash/ensnode
Length of output: 1888
🏁 Script executed:
#!/bin/bash
# Check root package.json for these tools
echo "=== Checking root package.json ==="
cat package.json | head -100
Repository: namehash/ensnode
Length of output: 2512
🏁 Script executed:
#!/bin/bash
# Search for CI configuration files
echo "=== Searching for CI config files ==="
find . -type f \( -name "*.yml" -o -name "*.yaml" -o -name ".github" \) -path '*/\.github/*' 2>/dev/null | head -20
git ls-files | grep -E '(\.github|\.gitlab|\.circleci|\.jenkins)'
Repository: namehash/ensnode
Length of output: 1988
🏁 Script executed:
#!/bin/bash
# Search for tsgo in devDependencies across package.json files
rg -A 3 '"tsgo"' --type json
Repository: namehash/ensnode
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check the main CI workflow file for typecheck command
cat .github/workflows/test_ci.yml | head -100
Repository: namehash/ensnode
Length of output: 3503
🏁 Script executed:
#!/bin/bash
# Check if tsgo is in pnpm-lock.yaml (it must be installed if it's being used)
rg -n 'tsgo' pnpm-lock.yaml | head -20
Repository: namehash/ensnode
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check for catalog configuration in pnpm-workspace.yaml or similar
cat pnpm-workspace.yaml 2>/dev/null || echo "File not found"
Repository: namehash/ensnode
Length of output: 752
🏁 Script executed:
#!/bin/bash
# Search for tsgo in all files to understand how it's being resolved
find . -type f -name "*.json" -o -name "*.yaml" -o -name "*.yml" | xargs grep -l '"tsgo"' 2>/dev/null | grep -v node_modules | head -20
Repository: namehash/ensnode
Length of output: 42
Typecheck script uses unavailable tsgo tool; generate:openapi script via tsx is correct.
The typecheck script references tsgo, but this tool is not found in the project's dependencies, lockfile (pnpm-lock.yaml), or catalog configuration. The generate:openapi script correctly uses tsx (version 4.20.6 installed). Add tsgo as a dev dependency across the workspace or switch to the standard TypeScript compiler (tsc).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/package.json` around lines 20 - 21, The package.json "typecheck"
script references the unavailable tool "tsgo" (scripts: "typecheck") which
causes CI/local failures; either add "tsgo" as a devDependency across the
workspace (pin a compatible version) and update lockfiles, or change the
"typecheck" script to use the standard TypeScript compiler ("tsc") instead
(matching how "generate:openapi" uses "tsx"); update package.json's "typecheck"
entry accordingly and run install to ensure the tool is present.
| responses: { | ||
| 200: { | ||
| description: | ||
| "Indexing progress is guaranteed to be within the requested distance of realtime", | ||
| }, | ||
| 503: { | ||
| description: | ||
| "Indexing progress is not guaranteed to be within the requested distance of realtime or indexing status unavailable", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
200 and 503 response bodies are undocumented in the spec.
Both responses declare only description with no content / schema. However the handler (amirealtime-api.ts) always returns JSON:
- 200:
{ maxWorstCaseDistance, slowestChainIndexingCursor, worstCaseDistance } - 503:
{ message: string }(viaerrorResponse)
The OpenAPI spec will incorrectly indicate body-less responses for both status codes.
✏️ Proposed fix (add response schemas)
+import { z } from "zod/v4";
export const getAmIRealtimeRoute = createRoute({
...
responses: {
description:
"Indexing progress is guaranteed to be within the requested distance of realtime",
+ content: {
+ "application/json": {
+ schema: z.object({
+ maxWorstCaseDistance: z.number(),
+ slowestChainIndexingCursor: z.number().nullable(),
+ worstCaseDistance: z.number(),
+ }),
+ },
+ },
},
description:
"Indexing progress is not guaranteed to be within the requested distance of realtime or indexing status unavailable",
+ content: {
+ "application/json": {
+ schema: z.object({ message: z.string() }),
+ },
+ },
},
},
});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| responses: { | |
| 200: { | |
| description: | |
| "Indexing progress is guaranteed to be within the requested distance of realtime", | |
| }, | |
| 503: { | |
| description: | |
| "Indexing progress is not guaranteed to be within the requested distance of realtime or indexing status unavailable", | |
| }, | |
| }, | |
| responses: { | |
| 200: { | |
| description: | |
| "Indexing progress is guaranteed to be within the requested distance of realtime", | |
| content: { | |
| "application/json": { | |
| schema: z.object({ | |
| maxWorstCaseDistance: z.number(), | |
| slowestChainIndexingCursor: z.number().nullable(), | |
| worstCaseDistance: z.number(), | |
| }), | |
| }, | |
| }, | |
| }, | |
| 503: { | |
| description: | |
| "Indexing progress is not guaranteed to be within the requested distance of realtime or indexing status unavailable", | |
| content: { | |
| "application/json": { | |
| schema: z.object({ message: z.string() }), | |
| }, | |
| }, | |
| }, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/handlers/amirealtime-api.routes.ts` around lines 31 - 40, The
OpenAPI responses in amirealtime-api.routes.ts lack response bodies: update the
responses object for status 200 to include content: application/json with a
schema describing the success payload (properties: maxWorstCaseDistance,
slowestChainIndexingCursor, worstCaseDistance with appropriate types) and for
503 add content: application/json with a schema matching the errorResponse shape
(message: string); ensure these schemas align with the handler
amirealtime-api.ts return values so the spec accurately documents the JSON
payloads.
| responses: { | ||
| 200: { | ||
| description: "Successfully resolved records", | ||
| content: { | ||
| "application/json": { | ||
| schema: makeResolvePrimaryNamesResponseSchema(), | ||
| }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Copy-paste error: 200 description for getResolvePrimaryNamesRoute says "records".
The description on line 89 reads "Successfully resolved records" — copied from getResolveRecordsRoute. It should reference primary names.
✏️ Proposed fix
- description: "Successfully resolved records",
+ description: "Successfully resolved primary names",
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| responses: { | |
| 200: { | |
| description: "Successfully resolved records", | |
| content: { | |
| "application/json": { | |
| schema: makeResolvePrimaryNamesResponseSchema(), | |
| }, | |
| }, | |
| }, | |
| }, | |
| responses: { | |
| 200: { | |
| description: "Successfully resolved primary names", | |
| content: { | |
| "application/json": { | |
| schema: makeResolvePrimaryNamesResponseSchema(), | |
| }, | |
| }, | |
| }, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/handlers/resolution-api.routes.ts` around lines 88 - 97, The
response description in getResolvePrimaryNamesRoute is a copy-paste that says
"Successfully resolved records"; update the 200 response description to
reference primary names (e.g. "Successfully resolved primary names") in the
route definition that uses makeResolvePrimaryNamesResponseSchema() so the
description matches the handler's purpose.
| export const openapiDocumentation = { | ||
| openapi: "3.1.0" as const, | ||
| info: { | ||
| title: "ENSApi APIs", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Nitpick: "ENSApi APIs" in title is redundant.
ENSApi already contains "Api"; the title reads as "ENS API APIs". Consider "ENSNode APIs" or "ENSApi".
✏️ Proposed fix
- title: "ENSApi APIs",
+ title: "ENSNode APIs",
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| title: "ENSApi APIs", | |
| title: "ENSNode APIs", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/openapi.ts` at line 4, The OpenAPI title string is redundant
("ENSApi APIs"); update the title property in openapi.ts (the title symbol) to a
non-redundant value such as "ENSApi" or "ENSNode APIs" so it no longer reads
"API APIs" (replace the existing "ENSApi APIs" value with the chosen string).
| for (const route of group.routes) { | ||
| const fullPath = route.path === "/" ? group.basePath : `${group.basePath}${route.path}`; | ||
| // Override path to include the basePath prefix | ||
| app.openapi({ ...route, path: fullPath } as any, stub); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
as any on the route object silences type errors from the full OpenAPI route metadata.
The RouteGroup.routes type is widened to readonly { method: string; path: string }[], which discards all compile-time guarantees about the route shape; the subsequent as any cast propagates that loss into app.openapi(). A tighter approach is to preserve the original route type (e.g., ReturnType<typeof createRoute>) in RouteGroup, eliminating the need for either as any.
♻️ Proposed fix
+import type { RouteConfig } from "@hono/zod-openapi";
+
type RouteGroup = {
basePath: string;
- routes: readonly { method: string; path: string }[];
+ routes: readonly RouteConfig[];
};
...
- app.openapi({ ...route, path: fullPath } as any, stub);
+ app.openapi({ ...route, path: fullPath }, stub as any);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/routes.ts` at line 71, The route object is being cast with
`as any` when calling `app.openapi({ ...route, path: fullPath } as any, stub)`
because `RouteGroup.routes` was widened and lost the original route type;
restore a precise type for `RouteGroup.routes` (e.g.,
`ReadonlyArray<ReturnType<typeof createRoute>>` or the exact route return type
used by your router) so the `route` variable carries full OpenAPI metadata, then
remove the `as any` cast and call `app.openapi({ ...route, path: fullPath },
stub)` directly; update any related types/signatures (e.g., the `createRoute`
factory and `RouteGroup` definition) so TypeScript infers the proper shape
without needing casts.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pnpm-lock.yaml
Outdated
There was a problem hiding this comment.
@hono/zod-openapi@1.0.0-beta.1 declares a zod peer dependency of ^3.25.0, but this repo is using Zod v4 (e.g. zod@4.3.6 in the lockfile). This can produce peer dependency warnings or fail installs in stricter environments; consider switching to a version that declares ^4, or add an explicit pnpm override/peer dep rule to acknowledge Zod v4 compatibility for this package.
| zod: ^3.25.0 | |
| zod: ^3.25.0 || ^4.0.0 |
| app.route(nameTokensBasePath.replace(basePath, ""), nameTokensApi); | ||
|
|
||
| // Registrar Actions API | ||
| app.route("/registrar-actions", registrarActionsApi); | ||
| app.route(registrarActionsBasePath.replace(basePath, ""), registrarActionsApi); | ||
|
|
||
| // Resolution API | ||
| app.route("/resolve", resolutionApi); | ||
| app.route(resolutionBasePath.replace(basePath, ""), resolutionApi); |
There was a problem hiding this comment.
Using String.prototype.replace(basePath, "") to derive the child mount path is fragile: it will remove the first occurrence even if basePath is not a strict prefix, and it silently produces an incorrect mount path if basePath ever changes format (e.g. trailing slash) or appears elsewhere in the string. Prefer an explicit prefix check (e.g. startsWith) and slice, and throw if the expected prefix is missing to avoid accidental route mis-mounting.
|
|
||
| const app = createRoutesForSpec(); | ||
| const spec = app.getOpenAPI31Document(openapiDocumentation); | ||
|
|
There was a problem hiding this comment.
openapiDocumentation.info.version is intentionally a placeholder ("0.0.0"), but this script uses it as-is when generating the OpenAPI document. That means CI/docs-generated specs will have a different version than the runtime /openapi.json (which injects packageJson.version). Consider updating the script to set info.version from apps/ensapi/package.json (or accept a CLI/env override) so generated specs match runtime output.
| const app = createRoutesForSpec(); | |
| const spec = app.getOpenAPI31Document(openapiDocumentation); | |
| import * as fs from "fs"; | |
| import * as path from "path"; | |
| const app = createRoutesForSpec(); | |
| const packageJsonPath = path.join(__dirname, "..", "package.json"); | |
| const packageJsonRaw = fs.readFileSync(packageJsonPath, "utf8"); | |
| const packageJson = JSON.parse(packageJsonRaw) as { version?: string }; | |
| const resolvedVersion = | |
| process.env.OPENAPI_VERSION_OVERRIDE || | |
| packageJson.version || | |
| openapiDocumentation.info?.version; | |
| const openapiDocumentationWithVersion = { | |
| ...openapiDocumentation, | |
| info: { | |
| ...openapiDocumentation.info, | |
| version: resolvedVersion, | |
| }, | |
| }; | |
| const spec = app.getOpenAPI31Document(openapiDocumentationWithVersion); |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts`:
- Around line 20-37: Refactor to remove duplicated page/recordsPerPage
validation by reusing a shared pagination schema: create or export
paginationQuerySchema (matching the existing page and recordsPerPage zod shapes)
from ensanalytics-api.routes.ts (or a shared module) and then change
referrerLeaderboardPageQuerySchema to extend/import that schema and add edition
via makeReferralProgramEditionSlugSchema("edition"); ensure the shared schema
uses the same REFERRERS_PER_LEADERBOARD_PAGE_MAX constant and update the import
references so referrerLeaderboardPageQuerySchema uses
paginationQuerySchema.extend({ edition: ... }) (or equivalent) instead of
re-declaring page and recordsPerPage.
In `@apps/ensapi/src/handlers/ensanalytics-api.ts`:
- Around line 56-69: The catch blocks in ensanalytics-api.ts currently leak
error.message into the JSON response; keep the logger.error({ error }, ...)
as-is but stop forwarding error.message to clients: in both catch blocks replace
the conditional errorMessage construction and response payload so the serialized
return (via serializeReferrerLeaderboardPageResponse with
ReferrerLeaderboardPageResponseCodes.Error) uses a fixed generic message (e.g.,
"Internal server error" or "An unexpected error occurred") for the errorMessage
field rather than error.message, while retaining the detailed error only in
server logs.
- Around line 26-30: The invariant checks for c.var.referrerLeaderboard
currently throw before the try/catch, causing unstructured 500s; move the
undefined checks for referrerLeaderboard into the start of each handler's try
block (both referrerLeaderboardGetMeta and referrerDetailGetMeta) so that any
thrown Error is caught and serialized by the existing catch logic; specifically,
relocate the conditional that throws `Invariant(ensanalytics-api):
referrerLeaderboardMiddleware required` into the corresponding try block in each
handler and keep the rest of the handler logic unchanged so the catch can
produce the structured JSON error response.
In `@apps/ensapi/src/handlers/ensnode-api.routes.ts`:
- Around line 10-26: The OpenAPI spec for configGetMeta currently only documents
a 200 response but the route does a dynamic import (await import("@/config"))
which can fail; update the createRoute call for configGetMeta to add a 500 (or
5xx) response entry alongside 200 (similar to indexingStatusGetMeta's 503) that
describes an internal server error and optionally provides an error schema (or
application/json error message) to reflect failure during config loading; ensure
you reference configGetMeta and makeENSApiPublicConfigSchema when adding the new
responses block.
In `@apps/ensapi/src/handlers/ensnode-api.ts`:
- Around line 23-27: The handler currently does a dynamic import("@/config")
inside app.openapi on every /config request causing repeated async overhead;
hoist or cache the config import so it runs once: move the import to module
scope (or use a top-level await or a top-level cachedConfig variable initialized
once) and then in the app.openapi callback call buildEnsApiPublicConfig(config)
and serializeENSApiPublicConfig(…) using that cached config instead of
re-importing each request; update references to the dynamic import inside the
app.openapi handler (configGetMeta, buildEnsApiPublicConfig,
serializeENSApiPublicConfig) accordingly.
In `@apps/ensapi/src/handlers/name-tokens-api.ts`:
- Around line 78-83: The dynamic await import("@/config") in the name-tokens API
handler causes unnecessary async overhead on every request; replace it with a
cached module-scope config reference (or add a getConfig() memoizer) so the
config is imported only once and reused, then call
getIndexedSubregistries(config.namespace, config.ensIndexerPublicConfig.plugins)
using that cached config; update the code locations referencing the local
variable named config and preserve the existing use of getIndexedSubregistries
to avoid changing downstream logic.
In `@apps/ensapi/src/handlers/registrar-actions-api.routes.ts`:
- Around line 42-45: The withReferral field uses params.boolstring (defined as
z.string().pipe(z.enum(["true","false"])).transform(...)) but currently sets
.default(false), causing a type mismatch; change the chain on withReferral to
use .default("false") instead of .default(false) so the default value matches
params.boolstring's expected string input (keep .optional() and .describe
as-is).
In `@apps/ensapi/src/handlers/resolution-api.routes.ts`:
- Around line 14-44: The query schema in resolveRecordsGetMeta currently calls
params.selection.parse(...) inside the .transform callback which can throw a
ZodError outside the top-level validation; change this so all validation stays
in the schema layer by replacing the .transform parsing with a schema-level
composition (use .pipe to pipe the base object into params.selection for the
selection part) or, if you must keep the transform, replace .parse with
.safeParse and explicitly handle failures by throwing the safeParse error; refer
to resolveRecordsGetMeta, params.selection, params.selectionParams, .transform,
.pipe and .safeParse when making the change.
---
Duplicate comments:
In `@apps/ensapi/src/handlers/amirealtime-api.routes.ts`:
- Around line 31-40: The OpenAPI responses for the amirealtime endpoint
currently only include descriptions, but the handler returns JSON (via
c.json(...) for success and errorResponse(...) for failures), so update the
response entries in amirealtime-api.routes.ts to include a content section with
"application/json" and appropriate JSON schemas (or $ref) for both the 200
response (the progress object returned by c.json) and the 503 response (the
error shape returned by errorResponse); locate the responses block and add
content/schema entries matching the actual returned shapes so the spec reflects
the body payloads.
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts`:
- Around line 39-42: The referrerAddressSchema definition is duplicated; extract
it into a shared module and import it from both routes files instead of
redefining it. Create a shared export (e.g., export const referrerAddressSchema)
in a common file (shared schemas module) and replace the local definition in
ensanalytics-api-v1.routes.ts and ensanalytics-api.routes.ts with an import of
that shared referrerAddressSchema so both route files use the same symbol.
- Around line 1-14: The file imports Zod v4 via `import { z } from "zod/v4"`
while using `@hono/zod-openapi` (used by `createRoute`) which may only be Zod
v3-compatible; verify and align versions by either upgrading `@hono/zod-openapi`
to a Zod v4-compatible release (>= 1.2.x) in package.json and reinstalling, or
change to a Zod v3 import to match the existing `@hono/zod-openapi` version;
then run install and CI/build to ensure `import { z } from "zod/v4"` and the
`createRoute` usages compile without type/runtime errors.
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.ts`:
- Around line 40-46: The invariant checks currently throwing Errors (checking
c.var.referralLeaderboardEditionsCaches and similar) run before the try/catch
and produce unstructured 500s; move those checks inside the corresponding try
blocks in the async handlers (the app.openapi handler for
referralLeaderboardV1GetMeta and the other two handlers referenced) so they
throw inside try and get handled by the existing catch that returns structured
JSON; specifically locate checks referencing
c.var.referralLeaderboardEditionsCaches /
referralLeaderboardEditionsCachesMiddleware in the async (c) => { ... } handlers
and relocate them into the start of their try blocks (or replace with early
return via structured error response) to ensure errors are caught and formatted
consistently.
- Around line 107-121: The handler currently returns raw exception messages
(error.message) in 500 responses for the /v1/ensanalytics/referral-leaderboard
endpoint and the two other endpoints in this file; change these catch blocks so
they log the full error server-side via logger.error({ error }, "...") but
return a fixed, non-sensitive client-facing message (e.g. "Internal server
error" or a short generic errorMessage) in the serialize* response payloads
(e.g. where serializeReferrerLeaderboardPageResponse and related serializers are
used) and set the response code to the Error enum
(ReferrerLeaderboardPageResponseCodes.Error); ensure you remove or stop
forwarding error.message into the response body while keeping the existing error
logging calls.
- Around line 204-211: The filter that narrows editionLeaderboards to
validEditionLeaderboards is redundant because the earlier
uncachedEditions.length > 0 guard already returns when any leaderboard is an
Error; remove the filter call and instead treat editionLeaderboards as the
non-error type (either by assigning const validEditionLeaderboards =
editionLeaderboards or by casting to { editionSlug: ReferralProgramEditionSlug;
leaderboard: ReferrerLeaderboard }[]), and update any downstream usages to
reference validEditionLeaderboards (or use editionLeaderboards directly) so you
don't perform the unnecessary runtime filter; keep references to the symbols
editionLeaderboards, validEditionLeaderboards, uncachedEditions,
ReferrerLeaderboard, and ReferralProgramEditionSlug to locate the change.
In `@apps/ensapi/src/handlers/ensanalytics-api.routes.ts`:
- Around line 29-31: referrerAddressSchema is duplicated; extract it into a
shared module and import it where needed: create a new shared schema export
(e.g., export const referrerAddressSchema = z.object({ referrer:
makeLowercaseAddressSchema("Referrer address").describe("Referrer Ethereum
address") });) in a central schemas file, update the current file to remove the
local definition and import referrerAddressSchema from the new module, and also
replace the duplicate definition in enanalytics-api-v1.routes.ts to import the
same shared symbol so both routes use the single source of truth.
In `@apps/ensapi/src/handlers/ensnode-api.ts`:
- Around line 54-60: The route mounting is using fragile string.replace calls;
update app.route invocations to derive mount paths by slicing off the prefix
(use nameTokensBasePath.slice(basePath.length),
registrarActionsBasePath.slice(basePath.length),
resolutionBasePath.slice(basePath.length)) and ensure each source string
actually startsWith(basePath) before slicing (throw or log an error if not) so
the intent is explicit and boundary-safe when calling app.route(...).
In `@apps/ensapi/src/handlers/name-tokens-api.routes.ts`:
- Around line 38-80: Extract the repeated schema call into a single constant:
create a const (e.g., nameTokensResponseSchema) assigned to
makeNameTokensResponseSchema("Name Tokens Response", true) and replace the three
occurrences inside the responses object (the 200, 404, and 503 response entries)
with that constant; update imports/usages if necessary so the responses map
references nameTokensResponseSchema instead of calling
makeNameTokensResponseSchema each time.
In `@apps/ensapi/src/handlers/name-tokens-api.ts`:
- Around line 109-111: The error message passed to
makeNameTokensNotIndexedResponse has an unclosed single quote in the template
string (`This ENSNode instance has not been configured to index tokens for the
requested name: '${name}`); fix it by closing the quote after the interpolation
so the string becomes `'...requested name: '${name}''` (i.e., add the missing
trailing single quote) in the call to makeNameTokensNotIndexedResponse to ensure
the message string is properly terminated.
- Line 43: The handler declaration passed to app.openapi for nameTokensGetMeta
uses an explicit ": Promise<any>" return type which disables `@hono/zod-openapi`
response validation; remove the explicit ": Promise<any>" from the async handler
signature so TypeScript infers the correct return type (keep the async (c) => {
... } structure) and ensure the returned value matches the route's declared
response schemas.
In `@apps/ensapi/src/handlers/registrar-actions-api.routes.ts`:
- Around line 28-40: The query param schemas `page` and `recordsPerPage` use
params.queryParam (a string schema) but set numeric defaults (`.default(1)` and
`.default(RECORDS_PER_PAGE_DEFAULT)`), which relies on Zod v4 short-circuiting
and is fragile; change those defaults to string values (e.g., `"1"` and
`String(RECORDS_PER_PAGE_DEFAULT)`) so the subsequent `.pipe(z.coerce.number())`
always runs, keeping validation via `makePositiveIntegerSchema` and the
`.max(RECORDS_PER_PAGE_MAX)` intact for `page`, `recordsPerPage`,
`params.queryParam`, `makePositiveIntegerSchema`, and
`RECORDS_PER_PAGE_DEFAULT`.
In `@apps/ensapi/src/handlers/resolution-api.routes.ts`:
- Around line 88-97: The response description for the resolvePrimaryNamesGetMeta
route is a copy-paste error: replace the incorrect text "Successfully resolved
records" with "Successfully resolved primary names" in the OpenAPI responses
object for the route that uses makeResolvePrimaryNamesResponseSchema(); update
the 200.description string associated with that route (look for
resolvePrimaryNamesGetMeta / makeResolvePrimaryNamesResponseSchema in
resolution-api.routes.ts) so it accurately describes primary names.
| export const referrerLeaderboardPageQuerySchema = z.object({ | ||
| edition: makeReferralProgramEditionSlugSchema("edition"), | ||
| page: z | ||
| .optional(z.coerce.number().int().min(1, "Page must be a positive integer")) | ||
| .describe("Page number for pagination"), | ||
| recordsPerPage: z | ||
| .optional( | ||
| z.coerce | ||
| .number() | ||
| .int() | ||
| .min(1, "Records per page must be at least 1") | ||
| .max( | ||
| REFERRERS_PER_LEADERBOARD_PAGE_MAX, | ||
| `Records per page must not exceed ${REFERRERS_PER_LEADERBOARD_PAGE_MAX}`, | ||
| ), | ||
| ) | ||
| .describe("Number of referrers per page"), | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
page and recordsPerPage validation is duplicated with ensanalytics-api.routes.ts.
The page and recordsPerPage sub-schemas (lines 22–36) are identical to paginationQuerySchema in ensanalytics-api.routes.ts (lines 11–26). Consider importing/reusing the pagination fields from a shared location. The only difference is that this schema adds the required edition field, so a spread or .extend() on a shared base would keep them in sync.
Sketch
In a shared location (or in ensanalytics-api.routes.ts):
export const paginationQuerySchema = z.object({
page: z.optional(z.coerce.number().int().min(1, "Page must be a positive integer"))
.describe("Page number for pagination"),
recordsPerPage: z.optional(
z.coerce.number().int()
.min(1, "Records per page must be at least 1")
.max(REFERRERS_PER_LEADERBOARD_PAGE_MAX, `Records per page must not exceed ${REFERRERS_PER_LEADERBOARD_PAGE_MAX}`)
).describe("Number of referrers per page"),
});
Then in ensanalytics-api-v1.routes.ts:
import { paginationQuerySchema } from "./ensanalytics-api.routes";
export const referrerLeaderboardPageQuerySchema = paginationQuerySchema.extend({
edition: makeReferralProgramEditionSlugSchema("edition"),
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts` around lines 20 - 37,
Refactor to remove duplicated page/recordsPerPage validation by reusing a shared
pagination schema: create or export paginationQuerySchema (matching the existing
page and recordsPerPage zod shapes) from ensanalytics-api.routes.ts (or a shared
module) and then change referrerLeaderboardPageQuerySchema to extend/import that
schema and add edition via makeReferralProgramEditionSlugSchema("edition");
ensure the shared schema uses the same REFERRERS_PER_LEADERBOARD_PAGE_MAX
constant and update the import references so referrerLeaderboardPageQuerySchema
uses paginationQuerySchema.extend({ edition: ... }) (or equivalent) instead of
re-declaring page and recordsPerPage.
| app.openapi(referrerLeaderboardGetMeta, async (c) => { | ||
| // context must be set by the required middleware | ||
| if (c.var.referrerLeaderboard === undefined) { | ||
| throw new Error(`Invariant(ensanalytics-api): referrerLeaderboardMiddleware required`); | ||
| } |
There was a problem hiding this comment.
Invariant checks throw outside try — will produce unstructured 500s.
Same pattern as in ensanalytics-api-v1.ts: the invariant throws at lines 28–30 and 76–78 precede the try block. If the middleware is misconfigured, the raw Error bypasses your structured JSON error handling. Move these inside the try blocks so the catch can serialize a proper response.
Proposed fix (referrerLeaderboardGetMeta handler)
app.openapi(referrerLeaderboardGetMeta, async (c) => {
- // context must be set by the required middleware
- if (c.var.referrerLeaderboard === undefined) {
- throw new Error(`Invariant(ensanalytics-api): referrerLeaderboardMiddleware required`);
- }
-
try {
+ // context must be set by the required middleware
+ if (c.var.referrerLeaderboard === undefined) {
+ throw new Error(`Invariant(ensanalytics-api): referrerLeaderboardMiddleware required`);
+ }
+
if (c.var.referrerLeaderboard instanceof Error) {
Apply the same change to the referrerDetailGetMeta handler at lines 74–78.
Also applies to: 74-78
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/handlers/ensanalytics-api.ts` around lines 26 - 30, The
invariant checks for c.var.referrerLeaderboard currently throw before the
try/catch, causing unstructured 500s; move the undefined checks for
referrerLeaderboard into the start of each handler's try block (both
referrerLeaderboardGetMeta and referrerDetailGetMeta) so that any thrown Error
is caught and serialized by the existing catch logic; specifically, relocate the
conditional that throws `Invariant(ensanalytics-api):
referrerLeaderboardMiddleware required` into the corresponding try block in each
handler and keep the rest of the handler logic unchanged so the catch can
produce the structured JSON error response.
| } catch (error) { | ||
| logger.error({ error }, "Error in /ensanalytics/referrers endpoint"); | ||
| const errorMessage = | ||
| error instanceof Error | ||
| ? error.message | ||
| : "An unexpected error occurred while processing your request"; | ||
| return c.json( | ||
| serializeReferrerLeaderboardPageResponse({ | ||
| responseCode: ReferrerLeaderboardPageResponseCodes.Error, | ||
| error: "Internal server error", | ||
| errorMessage, | ||
| } satisfies ReferrerLeaderboardPageResponse), | ||
| 500, | ||
| ); |
There was a problem hiding this comment.
Internal error.message leaked to client in 500 responses.
Both catch blocks (lines 58–61 and 104–107) forward error.message directly into the JSON response. This can expose sensitive internals (stack traces, file paths, DB details). Return a fixed generic message to the client and keep the actual error server-side in the logger call.
Proposed fix (apply to both catch blocks)
} catch (error) {
logger.error({ error }, "Error in /ensanalytics/referrers endpoint");
- const errorMessage =
- error instanceof Error
- ? error.message
- : "An unexpected error occurred while processing your request";
return c.json(
serializeReferrerLeaderboardPageResponse({
responseCode: ReferrerLeaderboardPageResponseCodes.Error,
error: "Internal server error",
- errorMessage,
+ errorMessage: "An unexpected error occurred while processing your request",
} satisfies ReferrerLeaderboardPageResponse),
500,
);
}
Also applies to: 102-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/handlers/ensanalytics-api.ts` around lines 56 - 69, The catch
blocks in ensanalytics-api.ts currently leak error.message into the JSON
response; keep the logger.error({ error }, ...) as-is but stop forwarding
error.message to clients: in both catch blocks replace the conditional
errorMessage construction and response payload so the serialized return (via
serializeReferrerLeaderboardPageResponse with
ReferrerLeaderboardPageResponseCodes.Error) uses a fixed generic message (e.g.,
"Internal server error" or "An unexpected error occurred") for the errorMessage
field rather than error.message, while retaining the detailed error only in
server logs.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/src/app.ts`:
- Around line 45-46: The middleware indexingStatusMiddleware is currently
registered globally via app.use(indexingStatusMiddleware) causing unnecessary
work for endpoints like "/", "/health", and "/openapi.json"; change the
registration to only apply to API routes (for example register it via
app.use("/api", indexingStatusMiddleware) or attach it to the specific router(s)
that need c.var.indexingStatus) so only requests under the API path run the
cache/realtime projection logic.
- Around line 95-98: The comment above the /health route is stale: either remove
it or actually attach the middleware that can return 503; update
app.get("/health", ...) to include ensIndexerPublicConfigMiddleware (or the
middleware that implements the 503 behavior) before the handler, or
delete/replace the comment to reflect current behavior since
indexingStatusMiddleware only sets context and does not block; reference the
route handler for "/health", the indexingStatusMiddleware, and
ensIndexerPublicConfigMiddleware when making the change.
In `@apps/ensapi/src/handlers/name-tokens-api.ts`:
- Around line 44-57: Consolidate the two indexingStatus guards into one: replace
the separate checks for c.var.indexingStatus === undefined and
c.var.indexingStatus instanceof Error with a single guard that tests both
conditions (e.g., !c.var.indexingStatus || c.var.indexingStatus instanceof
Error) and returns the same serialized response using
serializeNameTokensResponse with responseCode NameTokensResponseCodes.Error and
errorCode NameTokensResponseErrorCodes.IndexingStatusUnsupported and a unified
error.details message that includes the specific condition (missing vs error) if
needed; update the handler around c.var.indexingStatus to use that single guard
and remove the duplicated block.
In `@apps/ensapi/src/openapi-routes.ts`:
- Around line 61-62: The stub handler currently types its parameter as any;
import and use Hono's Context type and annotate the stub's parameter accordingly
(i.e., change the signature of the stub function that returns c.text("") to
accept Context) so the handler is properly typed; update the import to include
Context and adjust the stub declaration (stub) to use that type.
- Line 69: The path concatenation for fullPath can produce double slashes when
group.basePath ends with '/' or route.path starts with '/', so update the
construction around fullPath to normalize inputs: ensure that if group.basePath
=== "/" you use route.path directly, otherwise strip any trailing '/' from
group.basePath and ensure route.path begins with a single leading '/' before
concatenating; reference the variables fullPath, group.basePath and route.path
in openapi-routes.ts and implement the normalization logic so resulting paths
never contain '//' (except the leading one).
---
Duplicate comments:
In `@apps/ensapi/package.json`:
- Around line 20-21: The "typecheck" npm script in package.json references the
CLI "tsgo" which may not be installed; either add "tsgo" as a devDependency or
update the script to use the TypeScript compiler (e.g., change the "typecheck"
script value to use "tsc --noEmit" or "tsc --noEmit -p tsconfig.json"); update
package.json's "typecheck" script accordingly and run npm install --save-dev
tsgo if you choose to add the dependency.
In `@apps/ensapi/src/app.ts`:
- Around line 69-71: Create a new routes module that exports a basePath constant
and reuse it instead of the hardcoded string: add a file (e.g.,
subgraph-api.routes.ts) that exports const basePath = "/subgraph" and any route
metadata, then update the app registration in apps/ensapi/src/app.ts to call
app.route(basePath, subgraphApi) (replace the existing app.route("/subgraph",
subgraphApi)); this keeps consistency with other routes while preserving the
subgraphApi handler.
In `@apps/ensapi/src/handlers/name-tokens-api.ts`:
- Around line 78-83: The dynamic await import("@/config") inside the handler
causes per-request async overhead; instead move or cache the config at module
scope: import or resolve config once and compute indexedSubregistries once at
module initialization so subsequent calls use the cached values. Locate the
current dynamic import and the indexedSubregistries assignment (symbols: config
and indexedSubregistries, and function getIndexedSubregistries) and refactor so
config is acquired at top-level (or stored in a module-scope promise/result) and
indexedSubregistries is computed once and reused by the handler.
- Line 43: The handler declaration for nameTokensGetMeta currently forces a
broad return type with "Promise<any>" which disables compile-time response
validation; remove the explicit ": Promise<any>" annotation from the async
function signature for nameTokensGetMeta so TypeScript infers the proper return
type from the route schema (i.e., change app.openapi(nameTokensGetMeta, async
(c): Promise<any> => { ... } to app.openapi(nameTokensGetMeta, async (c) => {
... }) ), ensuring the function body still returns values that match the route
schema.
In `@apps/ensapi/src/openapi-routes.ts`:
- Around line 46-49: The RouteGroup type widens route entries and loses OpenAPI
metadata; update the routes field to use RouteConfig from `@hono/zod-openapi`
(e.g., routes: readonly RouteConfig[]) instead of readonly { method: string;
path: string }[], import RouteConfig at the top, and remove the `as any` cast at
the usage site (the cast near Line 71) by making that value correctly typed as
RouteConfig[] so the OpenAPI schema metadata is preserved.
| // add ENSIndexer Indexing Status Middleware to all routes for convenience | ||
| app.use(indexingStatusMiddleware); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
indexingStatusMiddleware applied to all routes including /, /health, /openapi.json.
The global app.use(indexingStatusMiddleware) runs on every request, including routes that never inspect c.var.indexingStatus (root page, health check, OpenAPI doc). This adds unnecessary overhead (cache reads, realtime projections) to informational endpoints. Consider scoping it to API paths only, e.g., app.use("/api/*", indexingStatusMiddleware), or to the specific sub-apps that need it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/app.ts` around lines 45 - 46, The middleware
indexingStatusMiddleware is currently registered globally via
app.use(indexingStatusMiddleware) causing unnecessary work for endpoints like
"/", "/health", and "/openapi.json"; change the registration to only apply to
API routes (for example register it via app.use("/api",
indexingStatusMiddleware) or attach it to the specific router(s) that need
c.var.indexingStatus) so only requests under the API path run the cache/realtime
projection logic.
| // will automatically 503 if config is not available due to ensIndexerPublicConfigMiddleware | ||
| app.get("/health", async (c) => { | ||
| return c.json({ message: "fallback ok" }); | ||
| }); |
There was a problem hiding this comment.
Misleading comment on /health endpoint.
The comment on Line 95 says the endpoint "will automatically 503 if config is not available due to ensIndexerPublicConfigMiddleware," but no such middleware is applied in this file. The indexingStatusMiddleware (Line 46) sets a context variable but doesn't block or return 503. As written, /health always returns 200 with { message: "fallback ok" }.
Either remove the stale comment or wire the middleware that would actually produce 503 responses on this endpoint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/app.ts` around lines 95 - 98, The comment above the /health
route is stale: either remove it or actually attach the middleware that can
return 503; update app.get("/health", ...) to include
ensIndexerPublicConfigMiddleware (or the middleware that implements the 503
behavior) before the handler, or delete/replace the comment to reflect current
behavior since indexingStatusMiddleware only sets context and does not block;
reference the route handler for "/health", the indexingStatusMiddleware, and
ensIndexerPublicConfigMiddleware when making the change.
| // Invariant: context must be set by the required middleware | ||
| if (c.var.indexingStatus === undefined) { | ||
| return c.json( | ||
| serializeNameTokensResponse({ | ||
| responseCode: NameTokensResponseCodes.Error, | ||
| errorCode: NameTokensResponseErrorCodes.IndexingStatusUnsupported, | ||
| error: { | ||
| message: "Name Tokens API is not available yet", | ||
| details: "Indexing status middleware is required but not initialized.", | ||
| }, | ||
| }, | ||
| 503: { | ||
| description: | ||
| "Service unavailable - Name Tokens API prerequisites not met (indexing status not ready or required plugins not activated)", | ||
| content: { | ||
| "application/json": { | ||
| schema: validationResolver(makeNameTokensResponseSchema("Name Tokens Response", true)), | ||
| }, | ||
| }), | ||
| 503, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Two indexingStatus guard blocks return identical error codes — consider consolidating.
Both the undefined check (Lines 44-57) and the Error check (Lines 59-73) return the same errorCode (IndexingStatusUnsupported) and HTTP 503, differing only in the details message. These could be collapsed into a single guard that handles both states, reducing duplication.
♻️ Proposed consolidation
- if (c.var.indexingStatus === undefined) {
- return c.json(
- serializeNameTokensResponse({
- responseCode: NameTokensResponseCodes.Error,
- errorCode: NameTokensResponseErrorCodes.IndexingStatusUnsupported,
- error: {
- message: "Name Tokens API is not available yet",
- details: "Indexing status middleware is required but not initialized.",
- },
- }),
- 503,
- );
- }
-
- if (c.var.indexingStatus instanceof Error) {
- return c.json(
- serializeNameTokensResponse({
- responseCode: NameTokensResponseCodes.Error,
- errorCode: NameTokensResponseErrorCodes.IndexingStatusUnsupported,
- error: {
- message: "Name Tokens API is not available yet",
- details:
- "Indexing status has not yet reached the required state to enable the Name Tokens API.",
- },
- }),
- 503,
- );
- }
+ if (c.var.indexingStatus === undefined || c.var.indexingStatus instanceof Error) {
+ const details = c.var.indexingStatus === undefined
+ ? "Indexing status middleware is required but not initialized."
+ : "Indexing status has not yet reached the required state to enable the Name Tokens API.";
+ return c.json(
+ serializeNameTokensResponse({
+ responseCode: NameTokensResponseCodes.Error,
+ errorCode: NameTokensResponseErrorCodes.IndexingStatusUnsupported,
+ error: {
+ message: "Name Tokens API is not available yet",
+ details,
+ },
+ }),
+ 503,
+ );
+ }
Also applies to: 59-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/handlers/name-tokens-api.ts` around lines 44 - 57,
Consolidate the two indexingStatus guards into one: replace the separate checks
for c.var.indexingStatus === undefined and c.var.indexingStatus instanceof Error
with a single guard that tests both conditions (e.g., !c.var.indexingStatus ||
c.var.indexingStatus instanceof Error) and returns the same serialized response
using serializeNameTokensResponse with responseCode
NameTokensResponseCodes.Error and errorCode
NameTokensResponseErrorCodes.IndexingStatusUnsupported and a unified
error.details message that includes the specific condition (missing vs error) if
needed; update the handler around c.var.indexingStatus to use that single guard
and remove the duplicated block.
| // Stub handler for spec generation - never actually called | ||
| const stub = (c: any) => c.text(""); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Stub handler uses any — consider typing the context parameter.
Minor nit: the stub could use Context from Hono to avoid the any type, though this is non-critical since the stub is never executed at runtime.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/openapi-routes.ts` around lines 61 - 62, The stub handler
currently types its parameter as any; import and use Hono's Context type and
annotate the stub's parameter accordingly (i.e., change the signature of the
stub function that returns c.text("") to accept Context) so the handler is
properly typed; update the import to include Context and adjust the stub
declaration (stub) to use that type.
|
|
||
| for (const group of routeGroups) { | ||
| for (const route of group.routes) { | ||
| const fullPath = route.path === "/" ? group.basePath : `${group.basePath}${route.path}`; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Path construction can produce double slashes if basePath ends with /.
If any basePath is "/" or ends with "/" and route.path starts with "/", the concatenation will produce // in the resulting path (e.g., /api//status). While the current basePaths likely don't end with /, this is fragile.
♻️ Safer path join
- const fullPath = route.path === "/" ? group.basePath : `${group.basePath}${route.path}`;
+ const fullPath = route.path === "/"
+ ? group.basePath
+ : `${group.basePath.replace(/\/$/, "")}${route.path}`;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fullPath = route.path === "/" ? group.basePath : `${group.basePath}${route.path}`; | |
| const fullPath = route.path === "/" | |
| ? group.basePath | |
| : `${group.basePath.replace(/\/$/, "")}${route.path}`; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/openapi-routes.ts` at line 69, The path concatenation for
fullPath can produce double slashes when group.basePath ends with '/' or
route.path starts with '/', so update the construction around fullPath to
normalize inputs: ensure that if group.basePath === "/" you use route.path
directly, otherwise strip any trailing '/' from group.basePath and ensure
route.path begins with a single leading '/' before concatenating; reference the
variables fullPath, group.basePath and route.path in openapi-routes.ts and
implement the normalization logic so resulting paths never contain '//' (except
the leading one).
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
hono-openapito@hono/zod-openapi(beta, with Zod v4 support).describeRoute()+validate()middleware pattern withcreateRoute()+app.openapi()pattern..routes.tsfiles with zero config/env dependencies.validate.tswrapper. Validation is now handled byapp.openapi()with adefaultHookinhono-factory.ts.openapi.ts(documentation config) androutes.ts(lightweight route composition for spec generation).scripts/generate-openapi.tsandpnpm generate:openapiscript to generate the OpenAPI spec without booting the server.Why
The OpenAPI spec was only available at runtime via
GET /openapi.json, which requires a fully configured and running server. We need to generate it in CI and for docs/SDK generation without env vars or network access.The previous
hono-openapilibrary separates route metadata (describeRoute) from validation (validate), making it hard to extract route definitions without importing handler files that pull in config.@hono/zod-openapicombines route metadata and validation into a singlecreateRoute()object, which is pure data with no side effects and can be imported without triggering config resolution.Testing
pnpm typecheckpasses with no errors.pnpm testpasses (63/63 tests).pnpm lint:cipasses.pnpm generate:openapiproduces valid OpenAPI 3.1.0 JSON with all 14 paths (not committed, will be in another PR).Notes for Reviewer (Optional)
The runtime server (
index.ts) still mounts the real handler files directly.routes.tsandcreateRoutesForSpec()are only used by the generate script. Handler files import their route definitions from the.routes.tssiblings.@hono/zod-openapiis pinned to1.0.0-beta.1because the stable release (0.19.x) does not support Zod v4. The beta has Zod v4 support merged via honojs/middleware#1223.No changes to runtime behaviour.
Pre-Review Checklist (Blocking)