feat: improve wildcard api route #442
Conversation
…es on directory paths
Updated the test suite for the `v1_files` API to use a consistent response assertion style with `expect(response).toMatchResponse(...)`. This change improves readability and maintainability of the tests. Additionally, adjusted the handling of paths and added new tests for query and type filters.
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughConsolidates directory listing/filtering into the wildcard file route, extends parseUnicodeDirectory to accept basePath, adds DirectoryFilterOptions and applyDirectoryFiltersAndSort, introduces OpenAPI params, enables schema refinements enforcing leading/trailing slashes, removes the separate /search route and updates tests. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Wildcard as Wildcard Route
participant Unicode as Unicode Public
participant Parser as parseUnicodeDirectory
participant Filter as applyDirectoryFiltersAndSort
Client->>Wildcard: GET /v1/files/{wildcard}?query&pattern&type&sort&order
Wildcard->>Wildcard: Validate path (reject traversal)
Wildcard->>Unicode: GET /Public/{wildcard}
alt Unicode returns directory HTML
Unicode-->>Wildcard: 200 HTML
Wildcard->>Parser: parseUnicodeDirectory(html, basePath)
Parser-->>Wildcard: Entry[] (names/paths/lastModified/type)
Wildcard->>Filter: applyDirectoryFiltersAndSort(entries, options)
Filter-->>Wildcard: Filtered & Sorted Entry[]
Wildcard->>Wildcard: Build directory headers (stats, cache)
Wildcard-->>Client: 200 JSON (entries) + headers
else Unicode returns file stream
Unicode-->>Wildcard: 200 stream (file) or 404/5xx
Wildcard->>Wildcard: Infer Content-Type / disposition, stream response
Wildcard-->>Client: 200 stream + headers (no forwarded Content-Length for streamed GET)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
github-actions
bot
added
apps: api
🌏 Preview Deployments
Built from commit: 🤖 This comment will be updated automatically when you push new commits to this PR. |
📋 OpenAPI Schema AnalysisSummaryAPI Endpoints
Schema Components
Overall Status
Detailed Changes🔴 Removed Endpoints (1)
|
| Path | Methods | Description |
|---|---|---|
/api/v1/files/search |
GET |
No description |
📋 Schema Components Changes
Removed Schemas (1):
FileEntry
Modified Schemas (1):
FileEntryList
Important
This PR contains breaking changes that require maintainer approval.
Maintainers: Add the approve-breaking-openapi label to approve these changes.
🤖 This comment is automatically updated when you push new commits.
Greptile SummaryThis PR consolidates the search functionality into the wildcard API route, simplifying the API surface while adding powerful filtering and sorting capabilities. Key Changes:
Architecture Impact:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant API as Wildcard Route
participant Files as files.ts
participant Unicode as unicode.org
Client->>API: GET /api/v1/files/{path}?query=Uni&type=files&sort=name
API->>API: Validate path (no traversal)
API->>Unicode: GET /Public/{path}?F=2
Unicode-->>API: HTML directory listing
API->>Files: parseUnicodeDirectory(html, basePath)
Files-->>API: Entry[]
API->>Files: applyDirectoryFiltersAndSort(entries, options)
Files-->>API: Filtered & sorted Entry[]
API->>API: buildDirectoryHeaders(files)
API-->>Client: JSON response with headers
|
There was a problem hiding this comment.
Pull request overview
This PR consolidates the wildcard API route by removing the dedicated /search endpoint and integrating its functionality into the main wildcard route. It enables directory paths to end with trailing slashes and adds comprehensive filtering, sorting, and query capabilities to directory listings.
Key Changes
- Removed the separate
/api/v1/files/searchendpoint and integrated query-based prefix search into the wildcard route - Enhanced directory listing responses with query parameters for filtering (
query,pattern,type) and sorting (sort,order) - Updated the schema to enforce path conventions (directory paths end with
/, all paths start with/)
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
packages/schemas/src/fs.ts |
Enabled path validation requiring directories to end with / and all paths to start with / |
packages/schemas/test/fs.test.ts |
Updated tests to reflect new directory path format with trailing slashes |
apps/api/src/routes/v1_files/$wildcard.ts |
Added query/pattern/type/sort/order parameters, integrated directory filtering logic, added helper functions for building headers |
apps/api/src/routes/v1_files/openapi-params.ts |
New file defining OpenAPI parameter schemas for the enhanced wildcard route |
apps/api/src/routes/v1_files/router.ts |
Removed registration of the search route |
apps/api/src/routes/v1_files/search.ts |
Deleted - functionality moved to wildcard route |
apps/api/src/lib/files.ts |
Added applyDirectoryFiltersAndSort function with query/pattern/type/sort/order support; updated parseUnicodeDirectory to accept basePath |
apps/api/test/routes/v1_files/search.test.ts |
Deleted - tests migrated to wildcard test file |
apps/api/test/routes/v1_files/$wildcard.test.ts |
Added extensive tests for new query filter, type filter, and sorting features; updated existing tests to use new matcher helpers |
apps/api/test/unit/files.test.ts |
Updated tests for path handling with trailing slashes for directories |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/lib/files.ts (1)
6-34: Update JSDoc to document thebasePathparameter.The function signature now includes a
basePathparameter, but the JSDoc comment doesn't document it. The existing example and parameter documentation should be updated to reflect this change.🔎 Proposed JSDoc update
/** * Parses an HTML directory listing from Unicode.org and extracts file/directory entries. * * This function takes raw HTML content from a Unicode.org directory listing page * and converts it into a structured array of Entry objects containing metadata * about each file or subdirectory. * * @param {string} html - The raw HTML content from a Unicode.org directory listing page + * @param {string} basePath - Optional base path to prepend to entry paths (defaults to "") * @returns {Promise<Entry[]>} A promise that resolves to an array of Entry objects, each containing * type, name, path, and lastModified information for files/directories * * @example * ```typescript * const html = await fetch('https://unicode.org/Public?F=2').then(r => r.text()); - * const entries = await parseUnicodeDirectory(html); + * const entries = await parseUnicodeDirectory(html, '/15.1.0'); * console.log(entries); // [{ type: 'directory', name: 'UNIDATA', path: '/UNIDATA', ... }] * ``` */
🤖 Fix all issues with AI agents
In @apps/api/test/routes/v1_files/$wildcard.test.ts:
- Around line 869-905: The test "should sort by lastModified ascending"
currently verifies descending order and uses identical timestamps, so update the
verification loop in this test (the block using results, prev, and the for loop)
to assert ascending order (each subsequent results[i].lastModified should be
greater than or equal to the previous) and change the mock entries passed to
generateAutoIndexHtml to have distinct timestamps (e.g., Date.now() - 2000,
Date.now() - 1000, Date.now()) so the sort can be reliably checked; also update
the inline comment to reflect "Verify sorted by lastModified ascending".
- Around line 907-943: The test "should sort by lastModified descending" uses
identical timestamps (all Date.now()) and a confusing verification loop; change
the mock entries passed to generateAutoIndexHtml so each file has distinct
lastModified values (e.g., now, now-1000, now-2000) so descending order is
testable, and simplify the verification by initializing prev to
results[0].lastModified and iterating from i=1 to results.length-1 asserting
results[i].lastModified <= prev, updating prev each iteration; keep the rest of
the test flow (mockFetch, executeRequest, results checks) unchanged.
🧹 Nitpick comments (6)
apps/api/src/lib/files.ts (2)
36-61: Consider using union types for better type safety.The
type,sort, andorderfields are defined asstring, which allows any string value. Consider using union types to restrict to valid values and improve type safety:🔎 Proposed type safety improvements
export interface DirectoryFilterOptions { /** * A string to filter file/directory names that start with this query (case-insensitive). */ query?: string; /** * A glob pattern to filter file/directory names. */ pattern?: string; /** * Type of entries to include: "all" (default), "files", or "directories". */ - type?: string; + type?: "all" | "files" | "directories"; /** * Field to sort by: "name" (default) or "lastModified". */ - sort?: string; + sort?: "name" | "lastModified"; /** * Sort order: "asc" (default) or "desc". */ - order?: string; + order?: "asc" | "desc"; }
78-79: Use the existingcreateLoggerutility for consistency with other API routes.The codebase provides a
createLoggerutility (inapps/api/src/lib/logger.ts) that wraps console logging with a tag prefix. Other routes likev1_versionsalready use it. Apply the same pattern here by importingcreateLoggerand replacing the directconsole.infocalls with the logger instance for consistency across the API codebase.apps/api/src/routes/v1_files/openapi-params.ts (1)
131-131: Consider removing unnecessary type assertions.The
as string[]type assertions on the enum arrays may be unnecessary. TypeScript should infer the correct readonly tuple type from the array literals.🔎 Simplified enum definitions
schema: { type: "string", - enum: ["all", "files", "directories"] as string[], + enum: ["all", "files", "directories"], default: "all", }, // ... and similarly for other enums schema: { type: "string", - enum: ["name", "lastModified"] as string[], + enum: ["name", "lastModified"], default: "name", }, schema: { type: "string", - enum: ["asc", "desc"] as string[], + enum: ["asc", "desc"], default: "asc", },Also applies to: 163-163, 189-189
apps/api/src/routes/v1_files/$wildcard.ts (3)
220-247: HEAD route OpenAPI spec marks mutually exclusive headers as required.The HEAD route response spec marks both file-specific headers (
UCD_STAT_SIZE_HEADER) and directory-specific headers (UCD_STAT_CHILDREN_*) asrequired: true. However, these are mutually exclusive based on the resource type - file responses won't have children headers, and directory responses won't have size headers.Consider setting these as
required: false(matching the GET route) or adding clarifying documentation that these are conditional based on theX-UCD-Stat-Typeheader value.🔎 Proposed fix
[UCD_STAT_SIZE_HEADER]: { description: "The size of the file in bytes (only for files)", schema: { type: "string", }, - required: true, + required: false, }, [UCD_STAT_CHILDREN_HEADER]: { description: "Number of children (only for directories)", schema: { type: "string", }, - required: true, + required: false, }, [UCD_STAT_CHILDREN_FILES_HEADER]: { description: "Number of child files (only for directories)", schema: { type: "string", }, - required: true, + required: false, }, [UCD_STAT_CHILDREN_DIRS_HEADER]: { description: "Number of child directories (only for directories)", schema: { type: "string", }, - required: true, + required: false, },
327-332: Consider adding a timeout to the upstream fetch.The fetch to
unicode.orgdoesn't specify a timeout. If the upstream server is slow or unresponsive, this could cause requests to hang indefinitely. Consider usingAbortSignal.timeout()for resilience.🔎 Example with timeout
const response = await fetch(url, { method: "GET", headers: { "User-Agent": DEFAULT_USER_AGENT, }, + signal: AbortSignal.timeout(30_000), // 30 second timeout });You may also want to catch
AbortErrorand return an appropriate gateway timeout response.
405-416: Inconsistent header construction between HEAD and GET file responses.The GET file response builds headers inline (lines 405-412), while HEAD uses
buildFileHeaders(line 400). This could lead to header drift over time. Consider extracting common header logic or documenting why they differ (GET streams without Content-Length for chunked transfer, HEAD requires buffering).The current behavior is correct (GET omits Content-Length for streaming), but a brief comment explaining the intentional difference would aid maintainability.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/api/src/lib/files.tsapps/api/src/routes/v1_files/$wildcard.tsapps/api/src/routes/v1_files/openapi-params.tsapps/api/src/routes/v1_files/router.tsapps/api/src/routes/v1_files/search.tsapps/api/test/routes/v1_files/$wildcard.test.tsapps/api/test/routes/v1_files/search.test.tsapps/api/test/unit/files.test.tspackages/schemas/src/fs.tspackages/schemas/test/fs.test.ts
💤 Files with no reviewable changes (3)
- apps/api/test/routes/v1_files/search.test.ts
- apps/api/src/routes/v1_files/router.ts
- apps/api/src/routes/v1_files/search.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{test,spec}.?(c|m)[jt]s?(x)
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.?(c|m)[jt]s?(x): Import test utilities from #test-utils/* path aliases instead of @ucdjs/test-utils in test files to avoid build step requirements and prevent cyclic dependencies
Use testdir() from vitest-testdirs to create temporary test directories in filesystem tests
Use mockStoreApi() utility from #test-utils/mock-store for MSW-based mocking of the UCD API in test files
Files:
packages/schemas/test/fs.test.tsapps/api/test/unit/files.test.tsapps/api/test/routes/v1_files/$wildcard.test.ts
apps/api/test/**/*.{test,spec}.?(c|m)[jt]s?(x)
📄 CodeRabbit inference engine (AGENTS.md)
Place API app tests in apps/api/test/routes/** for endpoint tests and apps/api/test/unit/** for unit tests with mocks
Files:
apps/api/test/unit/files.test.tsapps/api/test/routes/v1_files/$wildcard.test.ts
apps/api/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Regenerate OpenAPI spec by running 'pnpm build:openapi' after changing API routes or Zod schemas
Files:
apps/api/src/lib/files.tsapps/api/src/routes/v1_files/openapi-params.tsapps/api/src/routes/v1_files/$wildcard.ts
apps/api/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/api/src/**/*.ts: Build API (apps/api) using Hono web framework with OpenAPI plugin (@hono/zod-openapi) and organize routes by version and type
Use Zod schemas for defining API route schemas and request/response types in the API application
Files:
apps/api/src/lib/files.tsapps/api/src/routes/v1_files/openapi-params.tsapps/api/src/routes/v1_files/$wildcard.ts
apps/api/src/routes/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Document API endpoints with OpenAPI annotations using @hono/zod-openapi decorators
Files:
apps/api/src/routes/v1_files/openapi-params.tsapps/api/src/routes/v1_files/$wildcard.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: luxass
Repo: ucdjs/ucd PR: 6
File: packages/cli/src/cmd/codegen/fields.ts:68-82
Timestamp: 2025-05-04T11:52:22.858Z
Learning: User prefers to address the issue with Node.js API compatibility (`Dirent.parentPath` not being part of Node's API and the `recursive: true` option for `readdir` being only available in Node 21+) in a separate PR rather than the current one.
📚 Learning: 2025-06-09T05:10:32.105Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 45
File: packages/ucd-store/src/download.ts:24-24
Timestamp: 2025-06-09T05:10:32.105Z
Learning: In the ucd-store package refactor, picomatch was moved from direct usage in download.ts to internal usage within the createPathFilter function in filter.ts. The pattern format is still picomatch-compatible, so JSDoc comments referencing picomatch pattern format remain correct.
Applied to files:
packages/schemas/test/fs.test.tsapps/api/test/unit/files.test.tsapps/api/test/routes/v1_files/$wildcard.test.tsapps/api/src/lib/files.tsapps/api/src/routes/v1_files/$wildcard.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to **/*.{test,spec}.?(c|m)[jt]s?(x) : Import test utilities from #test-utils/* path aliases instead of ucdjs/test-utils in test files to avoid build step requirements and prevent cyclic dependencies
Applied to files:
packages/schemas/test/fs.test.tsapps/api/test/unit/files.test.tsapps/api/test/routes/v1_files/$wildcard.test.ts
📚 Learning: 2025-05-04T11:52:22.858Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 6
File: packages/cli/src/cmd/codegen/fields.ts:68-82
Timestamp: 2025-05-04T11:52:22.858Z
Learning: User prefers to address the issue with Node.js API compatibility (`Dirent.parentPath` not being part of Node's API and the `recursive: true` option for `readdir` being only available in Node 21+) in a separate PR rather than the current one.
Applied to files:
packages/schemas/test/fs.test.tsapps/api/test/unit/files.test.tsapps/api/test/routes/v1_files/$wildcard.test.tsapps/api/src/lib/files.tspackages/schemas/src/fs.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to apps/api/test/**/*.{test,spec}.?(c|m)[jt]s?(x) : Place API app tests in apps/api/test/routes/** for endpoint tests and apps/api/test/unit/** for unit tests with mocks
Applied to files:
apps/api/test/routes/v1_files/$wildcard.test.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to **/*.{test,spec}.?(c|m)[jt]s?(x) : Use mockStoreApi() utility from #test-utils/mock-store for MSW-based mocking of the UCD API in test files
Applied to files:
apps/api/test/routes/v1_files/$wildcard.test.ts
📚 Learning: 2025-07-01T04:10:40.989Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 89
File: apps/api/src/routes/v1_files.ts:42-43
Timestamp: 2025-07-01T04:10:40.989Z
Learning: The `traverse` method from the `apache-autoindex-parse` library always returns an array, even when HTTP requests fail or encounter network errors. Network failures are handled internally by the library rather than throwing exceptions, so catch blocks are primarily for parsing errors or unexpected JavaScript runtime errors.
Applied to files:
apps/api/src/lib/files.ts
📚 Learning: 2025-07-09T15:05:57.763Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 97
File: packages/utils/src/fs-bridge/node.ts:40-50
Timestamp: 2025-07-09T15:05:57.763Z
Learning: The `parentPath` property on `fs.Dirent` was added in Node.js v20.12, contrary to previous assumptions about it not being part of the Node.js API.
Applied to files:
packages/schemas/src/fs.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to apps/api/src/routes/**/*.ts : Document API endpoints with OpenAPI annotations using hono/zod-openapi decorators
Applied to files:
apps/api/src/routes/v1_files/openapi-params.tsapps/api/src/routes/v1_files/$wildcard.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to apps/api/src/**/*.ts : Build API (apps/api) using Hono web framework with OpenAPI plugin (hono/zod-openapi) and organize routes by version and type
Applied to files:
apps/api/src/routes/v1_files/openapi-params.tsapps/api/src/routes/v1_files/$wildcard.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to apps/api/src/**/*.{ts,tsx} : Regenerate OpenAPI spec by running 'pnpm build:openapi' after changing API routes or Zod schemas
Applied to files:
apps/api/src/routes/v1_files/$wildcard.ts
🧬 Code graph analysis (4)
apps/api/test/unit/files.test.ts (1)
apps/api/src/lib/files.ts (1)
parseUnicodeDirectory(24-34)
apps/api/test/routes/v1_files/$wildcard.test.ts (5)
apps/api/test/helpers/request.ts (3)
text(36-38)executeRequest(15-40)json(33-35)packages/test-utils/src/msw.ts (1)
mockFetch(16-19)apps/api/wrangler-types.d.ts (1)
env(10237-10237)packages/env/src/constants.ts (2)
UCD_STAT_SIZE_HEADER(17-17)UCD_STAT_TYPE_HEADER(16-16)packages/env/src/index.ts (2)
UCD_STAT_SIZE_HEADER(5-5)UCD_STAT_TYPE_HEADER(6-6)
apps/api/src/lib/files.ts (1)
packages/ucd-store-v2/src/core/context.ts (1)
basePath(78-80)
apps/api/src/routes/v1_files/$wildcard.ts (7)
apps/api/src/openapi.ts (1)
OPENAPI_TAGS(7-11)apps/api/src/routes/v1_files/openapi-params.ts (6)
WILDCARD_PARAM(3-41)PATTERN_QUERY_PARAM(43-88)QUERY_PARAM(90-116)TYPE_QUERY_PARAM(118-148)SORT_QUERY_PARAM(150-175)ORDER_QUERY_PARAM(177-202)packages/env/src/constants.ts (5)
UCD_STAT_SIZE_HEADER(17-17)UCD_STAT_CHILDREN_HEADER(18-18)UCD_STAT_CHILDREN_FILES_HEADER(19-19)UCD_STAT_CHILDREN_DIRS_HEADER(20-20)UCD_STAT_TYPE_HEADER(16-16)packages/env/src/index.ts (6)
UCD_STAT_SIZE_HEADER(5-5)UCD_STAT_CHILDREN_HEADER(4-4)UCD_STAT_CHILDREN_FILES_HEADER(3-3)UCD_STAT_CHILDREN_DIRS_HEADER(2-2)UCD_STAT_TYPE_HEADER(6-6)DEFAULT_USER_AGENT(9-9)apps/api/src/constants.ts (2)
MAX_AGE_ONE_WEEK_SECONDS(13-13)HTML_EXTENSIONS(6-10)apps/api/src/routes/v1_files/utils.ts (2)
isInvalidPath(4-22)determineContentTypeFromExtension(32-38)apps/api/src/lib/files.ts (2)
parseUnicodeDirectory(24-34)applyDirectoryFiltersAndSort(70-125)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: Greptile Review
🔇 Additional comments (14)
packages/schemas/test/fs.test.ts (1)
11-11: LGTM! Test updates align with schema validation changes.The test paths have been correctly updated to include trailing slashes for directory entries, matching the new schema validation requirements enforced in
packages/schemas/src/fs.ts.Also applies to: 99-99
apps/api/test/unit/files.test.ts (2)
8-8: LGTM! Test updates correctly reflect the new parsing behavior.The changes accurately validate that:
- Directory paths preserve trailing slashes (e.g.,
/15.1.0/,/folder/)- Trailing slashes are trimmed from entry names only
- The test name update correctly reflects this distinction
Also applies to: 18-18, 29-29, 37-38
40-49: Good addition! Test coverage for leading slash trimming.This new test case properly validates that leading slashes are trimmed from directory names while the path preserves both leading and trailing slashes.
apps/api/src/lib/files.ts (2)
104-122: LGTM! Sorting implementation is well-designed.The sorting logic correctly:
- Prioritizes directories before files (like Windows File Explorer)
- Supports name and lastModified sorting
- Uses
localeComparewith numeric awareness for natural sorting (e.g., "2.0.0" < "10.0.0")- Handles null lastModified values safely with nullish coalescing
- Uses
toSorted()for immutable sorting
70-125: Integration is properly implemented with correct parameter passing and edge case handling.The route handler at apps/api/src/routes/v1_files/$wildcard.ts correctly passes query parameters to
applyDirectoryFiltersAndSortwith proper validation:
- Pattern validation using
isValidGlobPatternprevents malicious glob patterns (limits on length, segments, braces, stars, questions)- Query parameters are extracted via
c.req.query()and passed directly to the function- Optional parameters default safely within the function (
type="all",sort="name",order="asc")- OpenAPI schemas in openapi-params.ts ensure request validation
No actionable concerns.
apps/api/src/routes/v1_files/openapi-params.ts (4)
3-41: LGTM! Well-documented wildcard parameter.The parameter definition is comprehensive with clear descriptions, format options table, and relevant examples. The pattern
.*is appropriately permissive for a wildcard path parameter.
43-88: LGTM! Clear glob pattern documentation.The parameter definition provides excellent documentation with:
- Supported glob syntax table
- Practical examples
- Case-insensitive matching note
90-116: LGTM! Simple and clear query parameter.The prefix-based search parameter is well-documented with appropriate examples.
118-148: LGTM! Consistent parameter definitions with good defaults.All three filtering/sorting parameters (type, sort, order) follow a consistent structure with:
- Clear enum values
- Sensible defaults
- Helpful examples
- Comprehensive descriptions
Also applies to: 150-175, 177-202
packages/schemas/src/fs.ts (1)
62-78: Remove verification concern—the implementation is correct for Zod 4.The code properly uses
ctx.addIssue()withcode: "custom"withinsuperRefine(), which is the correct Zod 4 approach for custom validation. The unifiederrorparameter introduced in Zod 4 applies to schema-level error handling (e.g.,z.string({ error: "..." })), not to custom refinements. The validation logic is sound:
- Directories must end with
/- All paths must start with
/Minor note: Using
z.ZodIssueCode.custominstead of the string"custom"is slightly more robust, but both work correctly in Zod 4.3.5.apps/api/test/routes/v1_files/$wildcard.test.ts (1)
1-10: LGTM!The test file is well-structured with comprehensive coverage of the wildcard route functionality. Imports correctly use the
#test-utils/*path alias as per coding guidelines. The test organization follows the expected pattern for API endpoint tests.apps/api/src/routes/v1_files/$wildcard.ts (3)
270-298: LGTM!The header-building helper functions are well-structured, separating concerns for directory and file responses. They properly handle the different metadata headers for each resource type.
357-387: LGTM!Directory listing handling is well-implemented with proper HTML parsing, glob pattern validation, and filtering/sorting. The
applyDirectoryFiltersAndSortfunction gracefully handles invalid enum values with sensible defaults.
32-186: LGTM!The OpenAPI route definition is comprehensive with detailed parameter documentation, examples, and proper response schemas. The endpoint description clearly explains the directory listing features and query parameters available for filtering and sorting.
As per coding guidelines, remember to run
pnpm build:openapiafter these changes to regenerate the OpenAPI spec.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/api/src/routes/v1_files/$wildcard.ts:
- Around line 405-416: The GET file handler streams without buffering and
therefore omits UCD_STAT_SIZE_HEADER, causing inconsistency with HEAD which uses
buildFileHeaders to include size; fix by reading the upstream
response.headers.get("Content-Length") and, if present, set
headers[UCD_STAT_SIZE_HEADER] to that value before calling
c.newResponse(response.body, 200, headers), otherwise document in the OpenAPI
spec that UCD_STAT_SIZE_HEADER is only guaranteed for HEAD requests (or choose
to buffer via the same logic as buildFileHeaders if an accurate size is required
for GET).
🧹 Nitpick comments (2)
apps/api/src/routes/v1_files/$wildcard.ts (2)
260-264: Clarify Content-Length header description.The description "Byte length when applicable" is ambiguous, but the header is marked as
required: trueand is always present in the implementation. Consider updating the description to be more specific, such as "The size of the content in bytes" to match the actual behavior.
397-403: HEAD request buffers entire file for accurate size calculation.The implementation buffers the complete file (line 398) to determine accurate
Content-Length. While this ensures correctness, it may impact performance and memory usage for large files. Consider whether the upstreamContent-Lengthheader could be trusted directly, or document this as expected behavior for accuracy.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/src/routes/v1_files/$wildcard.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/api/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Regenerate OpenAPI spec by running 'pnpm build:openapi' after changing API routes or Zod schemas
Files:
apps/api/src/routes/v1_files/$wildcard.ts
apps/api/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/api/src/**/*.ts: Build API (apps/api) using Hono web framework with OpenAPI plugin (@hono/zod-openapi) and organize routes by version and type
Use Zod schemas for defining API route schemas and request/response types in the API application
Files:
apps/api/src/routes/v1_files/$wildcard.ts
apps/api/src/routes/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Document API endpoints with OpenAPI annotations using @hono/zod-openapi decorators
Files:
apps/api/src/routes/v1_files/$wildcard.ts
🧠 Learnings (5)
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to apps/api/src/routes/**/*.ts : Document API endpoints with OpenAPI annotations using hono/zod-openapi decorators
Applied to files:
apps/api/src/routes/v1_files/$wildcard.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to apps/api/src/**/*.ts : Build API (apps/api) using Hono web framework with OpenAPI plugin (hono/zod-openapi) and organize routes by version and type
Applied to files:
apps/api/src/routes/v1_files/$wildcard.ts
📚 Learning: 2025-06-09T05:10:32.105Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 45
File: packages/ucd-store/src/download.ts:24-24
Timestamp: 2025-06-09T05:10:32.105Z
Learning: In the ucd-store package refactor, picomatch was moved from direct usage in download.ts to internal usage within the createPathFilter function in filter.ts. The pattern format is still picomatch-compatible, so JSDoc comments referencing picomatch pattern format remain correct.
Applied to files:
apps/api/src/routes/v1_files/$wildcard.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to apps/api/src/**/*.{ts,tsx} : Regenerate OpenAPI spec by running 'pnpm build:openapi' after changing API routes or Zod schemas
Applied to files:
apps/api/src/routes/v1_files/$wildcard.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to apps/api/src/**/*.ts : Use Zod schemas for defining API route schemas and request/response types in the API application
Applied to files:
apps/api/src/routes/v1_files/$wildcard.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: typecheck
- GitHub Check: lint
- GitHub Check: test (windows-latest)
- GitHub Check: test (ubuntu-latest)
- GitHub Check: test (macos-latest)
🔇 Additional comments (6)
apps/api/src/routes/v1_files/$wildcard.ts (6)
81-108: Headers correctly marked as optional.The response headers are now properly configured with
required: falsefor mutually exclusive headers (size for files, children counts for directories), which accurately reflects that these headers are conditionally present based on resource type.
270-278: LGTM: Clean helper for directory headers.The helper function correctly constructs directory response headers with accurate child entry counts.
280-298: LGTM: Clean helper for file headers.The helper function correctly constructs file response headers including content type, size metadata, and optional Content-Disposition.
311-318: LGTM: Proper path traversal protection.Early path validation using
isInvalidPathprovides good security against path traversal attacks.
357-387: LGTM: Robust directory listing implementation.The directory handling properly parses HTML, validates glob patterns with security limits, applies filtering/sorting, and constructs appropriate response headers.
32-43: Reminder: Regenerate OpenAPI specification.After these schema and parameter changes, remember to regenerate the OpenAPI specification by running
pnpm build:openapi.As per coding guidelines: "Regenerate OpenAPI spec by running 'pnpm build:openapi' after changing API routes or Zod schemas"
🔗 Linked issue
📚 Description
This PR is improving the wildcard api route. It removes the old search endpoint, but makes it part of the wildcard route.
This helps close the #420.
Some tests are failing, but those are fixed in #439. But to avoid reviewing the same code twice, i will merge this #442, before the #439.
Summary by CodeRabbit
New Features
Refactor
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.