Refactor extensions v2 routes into modular files and centralize error/status handling - #183
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
api | 6bf4868 | Commit Preview URL Branch Preview URL |
Aug 03 2026, 07:22 PM |
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/services/extensions/v2/route-errors.ts">
<violation number="1" location="src/services/extensions/v2/route-errors.ts:1">
P3: Status handling remains duplicated across the v2 routes, so updating this centralized mapping will not update the endpoints that still use inline `NOT_FOUND ? 404 : 500` logic. Consolidating those call sites onto `statusFromErrorCode` would keep error-status behavior consistent as new database error codes are introduced.</violation>
</file>
<file name="src/services/extensions/v2/moderation-routes.ts">
<violation number="1" location="src/services/extensions/v2/moderation-routes.ts:366">
P3: The routing-order comment explaining why `GET /developers/{id}` must be registered after the static `GET /developers/*` routes survived the file split but now lives in the wrong file. It sits in `moderation-routes.ts` right after `unapprovedDevelopersRoute`, yet the wildcard it describes (`getDeveloperRoute`, and the `GET /developers/claims`, `/developers/claims/mine`, `/developers/unapproved` shadowing) is not defined anywhere in this file — it's defined in `developer-profile-routes.ts` and registered last via the call order in `index.ts` (`registerDeveloperProfileRoutes` is invoked last). As written, the comment reads as if the wildcard were registered here, which is misleading, and the actual invariant (that `registerDeveloperProfileRoutes` must stay the last registration call) is now enforced only implicitly by the call sequence in `index.ts` with no documentation near it. Suggest relocating this comment next to `getDeveloperRoute` in `developer-profile-routes.ts` (or adding a note at the registration block in `index.ts`) so the ordering constraint stays visible where it can actually be violated.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -0,0 +1,21 @@ | |||
| export function statusFromErrorCode(code?: string): 404 | 409 | 500 { | |||
There was a problem hiding this comment.
P3: Status handling remains duplicated across the v2 routes, so updating this centralized mapping will not update the endpoints that still use inline NOT_FOUND ? 404 : 500 logic. Consolidating those call sites onto statusFromErrorCode would keep error-status behavior consistent as new database error codes are introduced.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/services/extensions/v2/route-errors.ts, line 1:
<comment>Status handling remains duplicated across the v2 routes, so updating this centralized mapping will not update the endpoints that still use inline `NOT_FOUND ? 404 : 500` logic. Consolidating those call sites onto `statusFromErrorCode` would keep error-status behavior consistent as new database error codes are introduced.</comment>
<file context>
@@ -0,0 +1,21 @@
+export function statusFromErrorCode(code?: string): 404 | 409 | 500 {
+ if (code === "NOT_FOUND") return 404;
+ if (code === "CONFLICT") return 409;
</file context>
| } | ||
| return c.json({ result: data }, 200); | ||
| }); | ||
| // Registered after every other static-segment GET /developers/* route |
There was a problem hiding this comment.
P3: The routing-order comment explaining why GET /developers/{id} must be registered after the static GET /developers/* routes survived the file split but now lives in the wrong file. It sits in moderation-routes.ts right after unapprovedDevelopersRoute, yet the wildcard it describes (getDeveloperRoute, and the GET /developers/claims, /developers/claims/mine, /developers/unapproved shadowing) is not defined anywhere in this file — it's defined in developer-profile-routes.ts and registered last via the call order in index.ts (registerDeveloperProfileRoutes is invoked last). As written, the comment reads as if the wildcard were registered here, which is misleading, and the actual invariant (that registerDeveloperProfileRoutes must stay the last registration call) is now enforced only implicitly by the call sequence in index.ts with no documentation near it. Suggest relocating this comment next to getDeveloperRoute in developer-profile-routes.ts (or adding a note at the registration block in index.ts) so the ordering constraint stays visible where it can actually be violated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/services/extensions/v2/moderation-routes.ts, line 366:
<comment>The routing-order comment explaining why `GET /developers/{id}` must be registered after the static `GET /developers/*` routes survived the file split but now lives in the wrong file. It sits in `moderation-routes.ts` right after `unapprovedDevelopersRoute`, yet the wildcard it describes (`getDeveloperRoute`, and the `GET /developers/claims`, `/developers/claims/mine`, `/developers/unapproved` shadowing) is not defined anywhere in this file — it's defined in `developer-profile-routes.ts` and registered last via the call order in `index.ts` (`registerDeveloperProfileRoutes` is invoked last). As written, the comment reads as if the wildcard were registered here, which is misleading, and the actual invariant (that `registerDeveloperProfileRoutes` must stay the last registration call) is now enforced only implicitly by the call sequence in `index.ts` with no documentation near it. Suggest relocating this comment next to `getDeveloperRoute` in `developer-profile-routes.ts` (or adding a note at the registration block in `index.ts`) so the ordering constraint stays visible where it can actually be violated.</comment>
<file context>
@@ -0,0 +1,512 @@
+ }
+ return c.json({ result: data }, 200);
+ });
+ // Registered after every other static-segment GET /developers/* route
+ // (claims, claims/mine, unapproved above) — Hono matches path params against
+ // whichever handler was registered first among overlapping patterns, so this
</file context>
There was a problem hiding this comment.
0 issues found across 8 files (changes from recent commits).
Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
Description
DatabaseResult<T>insrc/lib/interfaces.tsto a discriminated union{ data: T; error: null } | { data: null; error: DatabaseError }to make result handling explicit.route-dependencies.tsto model shared dependencies (database,auth,platform,requireAuth,requireModerator) used by v2 routes.public-extensions-routes.ts,submission-routes.ts,ownership-routes.ts,moderation-routes.ts, anddeveloper-profile-routes.ts, and registered them fromsrc/services/extensions/v2/index.tsvia adependenciesobject.route-errors.tsto centralize mapping of internal error codes to HTTP status codes and updated route handlers to use these helpers; simplifiedrequireModeratorto handle the newDatabaseResultshape.index.tsby removing many inline route definitions and adjusted minor formatting in the Scalar call.