refactor: move /templates endpoints to a module#1498
refactor: move /templates endpoints to a module#1498stalniy merged 3 commits intoakash-network:mainfrom
Conversation
|
""" WalkthroughThe changes refactor the templates API routes by removing the old route implementations and introducing a new modular structure under a dedicated Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant Controller
participant GalleryService
participant FetcherService
participant ProcessorService
Client->>Router: GET /v1/templates
Router->>Controller: getTemplatesFull()
Controller->>GalleryService: getTemplateGallery()
GalleryService->>FetcherService: fetch templates from repos
FetcherService->>ProcessorService: process templates
FetcherService-->>GalleryService: categories[]
GalleryService-->>Controller: categories[]
Controller-->>Router: categories[]
Router-->>Client: JSON response
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
jzsfkzm
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (15)
apps/api/src/template/routes/index.ts (1)
1-1: Re-export template router for centralized route imports.
Consider switching to a relative path for consistency within this module:-export * from "@src/template/routes/templates/templates.router"; +export * from "./templates/templates.router";apps/api/test/mocks/templates/akash-network/awesome-akash/readme.md (1)
5-6: Reduce redundancy in list items.
The phrase "Cosmos SDK" appears twice. Consider renaming one entry to avoid repetition:- - [Cosmos SDK Node](https://github.com/ovrclk/akash-on-akash) + - [Akash Cosmos SDK Node](https://github.com/ovrclk/akash-on-akash)apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/readme.md (1)
3-3: Fix grammar in README title
Change “This make it easy” to “This makes it easy” for correct subject-verb agreement.apps/api/src/template/controllers/template/template.controller.ts (1)
11-13: Unnecessaryawaitbeforereturn
getTemplatesFull()already returns a promise; the extraawaitadds latency and an extra micro-task.- async getTemplatesFull() { - return await this.templateGalleryService.getTemplateGallery(); - } + async getTemplatesFull() { + return this.templateGalleryService.getTemplateGallery(); + }apps/api/test/functional/templates.spec.ts (1)
55-63: Guard against missing categories to improve test diagnosticsIf the searched category is absent,
categorybecomesundefinedand the next line throws a cryptic
TypeError: Cannot read property 'templates' of undefined.
Fail fast with an explicit assertion so the test output is self-explanatory.-const category = result.find((c: { title: string }) => c.title === expectedCategory); -expect(category.templates).toHaveLength(expectedTemplateIds.length); +const category = result.find((c: { title: string }) => c.title === expectedCategory); +expect(category).toBeDefined(); +expect(category!.templates).toHaveLength(expectedTemplateIds.length);apps/api/src/template/routes/templates/templates.router.ts (2)
30-34: Container resolution per request may defeat singleton scopes
container.resolve(TemplateController)is executed on every request, producing a new instance
because Tsyringe’s default lifetime is transient.If the controller/service tree holds cached state (e.g. memoised template gallery), you will
instantiate it repeatedly and lose the benefit.Either:
- Register
TemplateControlleras a singleton, or- Resolve it once and reuse:
const templateController = container.resolve(TemplateController); templatesRouter.openapi(getTemplatesFullRoute, c => c.json(templateController.getTemplatesFull(), 200) );
17-19: OpenAPI tag “Other” is vagueConsider a dedicated tag such as
Templatesto improve generated documentation navigation.apps/api/src/template/types/template.ts (1)
18-19:FinalCategoryname is ambiguous
FinalCategory = Omit<Category, "templateSources">hides the meaning of final.
Consider renaming toCategoryWithoutSourcesor documenting why this variant exists to aid maintainability.apps/api/src/template/http-schemas/template.schema.ts (1)
4-17: Tighten URL validation & clarify optionalitySeveral string fields clearly represent URLs but are validated only as plain strings, and
logoUrlis nullable yet not optional. Consider:- logoUrl: z.string().nullable(), - githubUrl: z.string(), + logoUrl: z.string().url().nullable().optional(), + githubUrl: z.string().url(), - config: z.object({ - ssh: z.boolean().optional() - }) + // make config itself optional to avoid forcing empty objects + config: z + .object({ + ssh: z.boolean().optional() + }) + .optional()This guards against malformed links and eases backwards-compatibility for clients that may omit optional fields.
apps/api/src/template/services/template-gallery/template-gallery.service.ts (3)
22-24: Annotate return type for public API
getTemplateGallery()is public and memoized yet lacks an explicit return type; addPromise<FinalCategory[]>to lock the contract and improve IDE help.- async getTemplateGallery() { + async getTemplateGallery(): Promise<FinalCategory[]> {
80-86: Usepath.joinfor cross-platform cache pathsString concatenation can break on Windows; prefer
path.join.- const cacheFilePath = `${dataFolderPath}/templates/${repoOwner}-${repoName}-${repoVersion}.json`; + const cacheFilePath = path.join( + dataFolderPath, + "templates", + `${repoOwner}-${repoName}-${repoVersion}.json` + );
97-99: Clean upgeneratingTasksto avoid key leakAfter the promise settles, delete the entry instead of assigning
null.- generatingTasks[cacheFilePath] = null; + delete generatingTasks[cacheFilePath];Keeps the map small over successive repo versions.
apps/api/src/template/services/template-fetcher/template-fetcher.service.ts (3)
112-120: Cache chain-registry look-ups
fetchChainRegistryDatafires a network request per template. For ~180+ chains this is slow and rate-limited. Memoize results (e.g., simple in-memoryMapkeyed bychainPath) or persist alongside template cache to cut latency.
229-231: Markdown parsing via regex is brittleThe dual-regex approach can break with minor README format changes (extra spaces, different heading levels, CRLF). Using a markdown parser such as
remarkto walk the AST will be more robust and easier to maintain.
142-146: Prefer typed errors over string throwingThrowing raw strings (
"Absolute URL") loses stack context and hinders error categorisation. Replace withnew Error("Absolute URL template path not supported")or a custom error class.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
apps/api/src/app.ts(2 hunks)apps/api/src/routes/v1/index.ts(0 hunks)apps/api/src/routes/v1/templates/byId.ts(0 hunks)apps/api/src/routes/v1/templates/list-full.ts(0 hunks)apps/api/src/routes/v1/templates/list.ts(0 hunks)apps/api/src/services/external/templateReposService.ts(0 hunks)apps/api/src/services/external/templatesCollector.ts(0 hunks)apps/api/src/template/controllers/template/template.controller.ts(1 hunks)apps/api/src/template/http-schemas/template.schema.ts(1 hunks)apps/api/src/template/index.ts(1 hunks)apps/api/src/template/routes/index.ts(1 hunks)apps/api/src/template/routes/templates/templates.router.ts(1 hunks)apps/api/src/template/services/template-fetcher/template-fetcher.service.ts(1 hunks)apps/api/src/template/services/template-gallery/template-gallery.service.ts(1 hunks)apps/api/src/template/services/template-processor/template-processor.service.ts(1 hunks)apps/api/src/template/types/template.ts(1 hunks)apps/api/test/functional/templates.spec.ts(1 hunks)apps/api/test/mocks/templates/akash-network/awesome-akash/alpaca-cpp/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/awesome-akash/lunie-lite/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/awesome-akash/readme.md(1 hunks)apps/api/test/mocks/templates/akash-network/awesome-akash/ssh-ubuntu/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/cosmos-omnibus/akash/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/cosmos-omnibus/archway/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/cosmos-omnibus/index.json(1 hunks)apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/adguardhome-sync/index.json(1 hunks)apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/airsonic-advanced/index.json(1 hunks)apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/airsonic/index.json(1 hunks)apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/readme.md(1 hunks)apps/api/tsconfig.strict.json(1 hunks)
💤 Files with no reviewable changes (6)
- apps/api/src/routes/v1/index.ts
- apps/api/src/routes/v1/templates/byId.ts
- apps/api/src/routes/v1/templates/list.ts
- apps/api/src/routes/v1/templates/list-full.ts
- apps/api/src/services/external/templateReposService.ts
- apps/api/src/services/external/templatesCollector.ts
🧰 Additional context used
🧬 Code Graph Analysis (5)
apps/api/src/app.ts (1)
apps/api/src/template/routes/templates/templates.router.ts (1)
templatesRouter(13-13)
apps/api/test/functional/templates.spec.ts (2)
apps/api/src/template/types/template.ts (1)
Category(11-16)apps/deploy-web/tests/fixture/context-with-extension.ts (1)
expect(77-77)
apps/api/src/template/routes/templates/templates.router.ts (2)
apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
OpenApiHonoHandler(11-26)apps/api/src/template/http-schemas/template.schema.ts (4)
GetTemplatesFullResponseSchema(24-24)GetTemplatesListResponseSchema(26-28)GetTemplateByIdParamsSchema(30-35)GetTemplateByIdResponseSchema(37-39)
apps/api/src/template/services/template-processor/template-processor.service.ts (3)
apps/api/src/template/services/template-gallery/template-gallery.service.ts (1)
injectable(15-106)apps/api/src/template/services/template-fetcher/template-fetcher.service.ts (1)
injectable(31-339)apps/api/src/template/types/template.ts (3)
Category(11-16)TemplateSource(1-9)Template(25-37)
apps/api/src/template/services/template-gallery/template-gallery.service.ts (4)
apps/api/src/template/types/template.ts (3)
Category(11-16)FinalCategory(18-18)Template(25-37)apps/api/src/template/services/template-fetcher/template-fetcher.service.ts (1)
REPOSITORIES(16-29)apps/api/src/caching/helpers.ts (1)
Memoize(22-38)apps/api/src/utils/constants.ts (1)
dataFolderPath(11-11)
🪛 LanguageTool
apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/readme.md
[grammar] ~3-~3: Possible subject-verb agreement error detected.
Context: ...nverted to akash compliant YAML files. This make it easy for new users to deploy more ap...
(THIS_THAT_AGR)
[duplication] ~12-~12: Possible typo: you repeated a word.
Context: ...AdGuardHome Sync - Airsonic - Airsonic Advanced
(ENGLISH_WORD_REPEAT_RULE)
apps/api/test/mocks/templates/akash-network/awesome-akash/readme.md
[grammar] ~5-~5: This phrase is duplicated. You should probably use “Cosmos SDK” only once.
Context: ... --> ### Official - Lunie Wallet for Cosmos SDK - [Cosmos SDK Node](https://github.com/ovrclk/akash-o...
(PHRASE_REPETITION)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: validate-api
- GitHub Check: test-api-build
🔇 Additional comments (15)
apps/api/src/template/index.ts (1)
1-1: Re-export template routes for streamlined imports.
This file centralizes all route exports under thetemplatemodule.apps/api/src/app.ts (2)
63-63: Import new templatesRouter to integrate template API routes.
ThetemplatesRouterimport aligns with the refactored modular structure undersrc/template.
139-139: Register templatesRouter on the main application router.
Ensure thattemplatesRouterapplies its internal prefixes (e.g./v1/templates) and does not conflict with existing routes.apps/api/tsconfig.strict.json (1)
32-32: Include newly added template sources in strict type checking.
Adding"src/template/**/*.ts"ensures all new template modules are covered under the strict TS configuration.apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/adguardhome-sync/index.json (1)
1-34: Mock data structure is correct and comprehensive.The JSON includes all expected GitHub API fields (
name,path,sha,size,url,html_url,git_url,download_url,type,_links) and matches the format used by other template fixtures. This will reliably support functional tests for theadguardhome-synctemplate.apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/airsonic-advanced/index.json (1)
1-35: Mock data is consistent and complete.All entries (
README.mdanddeploy.yaml) include the necessary metadata fields and the commit ref matches the othercryptoandcoffee/akash-linuxservermocks. This fixture aligns with the test harness expectations.apps/api/test/mocks/templates/akash-network/awesome-akash/lunie-lite/index.json (1)
1-67: Fixture correctly captures all lunie-lite files.Each file object (
README.md,config.json,deploy.yaml,lunie.png) contains the full suite of GitHub API fields, and the ordering mirrors the repository structure. This will adequately drive the template gallery service tests.apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/airsonic/index.json (1)
1-35: Airsonic mock matches expected schema.The two-file listing (
README.md&deploy.yaml) is formatted identically to other mocks under the same repo, ensuring consistency in the fetching and processing layers.apps/api/test/mocks/templates/akash-network/cosmos-omnibus/akash/index.json (1)
1-67: Cosmos-omnibus mock structure is accurate.All four template files are represented with complete GitHub metadata. This fixture aligns with the new
TemplateFetcherServiceexpectations for theakashdirectory.apps/api/test/mocks/templates/akash-network/cosmos-omnibus/archway/index.json (1)
1-66: Approve mock data structure
The JSON accurately mirrors the GitHub API response for thearchwaydirectory and provides all required file metadata for testing.apps/api/test/mocks/templates/akash-network/cosmos-omnibus/index.json (1)
1-83: Approve root directory fixture
This mock file correctly represents the top-level contents of thecosmos-omnibusrepo, ensuring full coverage for directory-listing tests.apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/readme.md (1)
12-12: Skip: false-positive duplication
The repeated word warning here is intentional (distinct entries). No change needed.apps/api/test/mocks/templates/akash-network/awesome-akash/alpaca-cpp/index.json (1)
1-50: Approve alpaca-cpp mock fixture
The JSON structure correctly lists thealpaca-cppdirectory contents with complete metadata for Dockerfile, README, and deploy.yaml.apps/api/test/mocks/templates/akash-network/awesome-akash/ssh-ubuntu/index.json (1)
1-99: Approve ssh-ubuntu mock data
This file provides a comprehensive and accurate mock of thessh-ubuntudirectory contents, supporting the new template fetching logic.apps/api/test/functional/templates.spec.ts (1)
15-16: Reset mutated env after suite to avoid cross-test leakage
env.GITHUB_PATis overwritten globally and never restored.
Down-stream suites that rely onenvmay read the mocked token unintentionally.
AddafterAll(() => { delete env.GITHUB_PAT; })or restore the original value.
| removeComments(markdown: string): string { | ||
| return markdown.replace(/<!--.+-->/g, ""); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
HTML comment removal misses multi-line comments
/<!--.+-->/g doesn’t match across newlines.
Use a non-greedy, dot-all expression:
-return markdown.replace(/<!--.+-->/g, "");
+return markdown.replace(/<!--[\s\S]*?-->/g, "");📝 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.
| removeComments(markdown: string): string { | |
| return markdown.replace(/<!--.+-->/g, ""); | |
| } | |
| removeComments(markdown: string): string { | |
| - return markdown.replace(/<!--.+-->/g, ""); | |
| + return markdown.replace(/<!--[\s\S]*?-->/g, ""); | |
| } |
🤖 Prompt for AI Agents
In
apps/api/src/template/services/template-processor/template-processor.service.ts
around lines 75 to 77, the regex used to remove HTML comments does not match
multi-line comments because the dot (.) does not match newline characters.
Update the regex to use a non-greedy match with a dot-all flag or an equivalent
pattern that matches across multiple lines, ensuring all HTML comments including
multi-line ones are removed correctly.
apps/api/src/template/services/template-processor/template-processor.service.ts
Outdated
Show resolved
Hide resolved
apps/api/src/template/services/template-processor/template-processor.service.ts
Outdated
Show resolved
Hide resolved
jzsfkzm
force-pushed
the
refactor/1273-templates
branch
2 times, most recently
from
f02ed01 to
052297c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1498 +/- ##
==========================================
+ Coverage 43.33% 44.42% +1.08%
==========================================
Files 883 885 +2
Lines 21689 21697 +8
Branches 3950 3954 +4
==========================================
+ Hits 9399 9638 +239
+ Misses 11593 11476 -117
+ Partials 697 583 -114
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
jzsfkzm
force-pushed
the
refactor/1273-templates
branch
from
052297c to
7d29a35
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (10)
apps/api/src/utils/json.ts (1)
1-7:as Tbypasses type safety; prefer parsing tounknownfirst
Casting the parsed value straight toTtrusts external input blindly. Consider returningunknownfromJSON.parseand letting callers validate/cast (or integrate a schema validator such as zod) to prevent runtime type mismatches (e.g., the JSON string"null"becomesnull, which is notT).[Nit, no blocking]
apps/api/test/mocks/templates/akash-network/awesome-akash/readme.md (1)
11-12: Add a Trailing Newline
Ending the file with a newline prevents potential markdown lint warnings and reduces diff noise on regeneration.apps/api/src/template/services/template-fetcher/template-fetcher.service.ts (4)
112-120: Uncaught network errors
fetchChainRegistryDatachecksresponse.statusbut does not catch network-level errors (DNS, timeout, etc.).
Wrap the call intry/ catchand convert to a domain error so the caller can decide whether to skip or abort.
142-146: Throwing a bare string
throw "Absolute URL";breaks stack traces and theerr.messagecheck below. Throw anErrorinstead.- throw "Absolute URL"; + throw new Error("Absolute URL");
164-191: Concurrency of 30 × (#categories) may exceed GitHub rate limits
processCategoryis invoked concurrently for every category, each of which internally spins up aPromisePoolof 30.
For repositories with many categories this fan-out can hit the 5 000 req/h REST limit quickly.Consider a single top-level limiter (e.g. p-limit) shared across the service or lower the pool size here to something conservative (~5-10).
327-337: Error aggregation drops stack tracesWhen you aggregate errors with
errors.map(e => e.message)you lose individual stack traces, which are invaluable when debugging.
Return/ log fulle.stack ?? e.messagefor each entry.apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/readme.md (1)
3-3: Grammar: “This make it easy” → “This makes it easy”.apps/api/scripts/buildAkashTemplatesCache.ts (2)
6-10:githubPATvariable is unusedThe script validates
GITHUB_PATbut then never passes it anywhere. Either remove the variable or forward it to the service for clarity:-const githubPAT = process.env.GITHUB_PAT; -if (!githubPAT) { +if (!process.env.GITHUB_PAT) { ...
12-12: Un-awaited promise
getTemplateGallery()returns a promise; withoutawait, a rejection may be missed before Node exits (depending on--unhandled-rejections). You already attach.catch, butawaitmakes the intent clearer:-await getTemplateGallery().catch(... +await getTemplateGallery().catch(...apps/api/src/services/external/templates-collector.ts (1)
7-10: Repeated instantiation ofTemplateGalleryServiceEach call to
getTemplateGallery()resolves a new transient instance. For an expensive, I/O-heavy service consider registering it as a singleton:container.registerSingleton(TemplateGalleryService);or cache the resolved instance inside this module.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
apps/api/scripts/buildAkashTemplatesCache.ts(2 hunks)apps/api/src/app.ts(2 hunks)apps/api/src/routes/v1/index.ts(0 hunks)apps/api/src/routes/v1/templates/byId.ts(0 hunks)apps/api/src/routes/v1/templates/list-full.ts(0 hunks)apps/api/src/routes/v1/templates/list.ts(0 hunks)apps/api/src/services/external/templateReposService.ts(0 hunks)apps/api/src/services/external/templates-collector.ts(1 hunks)apps/api/src/services/external/templatesCollector.ts(0 hunks)apps/api/src/template/controllers/template/template.controller.ts(1 hunks)apps/api/src/template/http-schemas/template.schema.ts(1 hunks)apps/api/src/template/index.ts(1 hunks)apps/api/src/template/routes/index.ts(1 hunks)apps/api/src/template/routes/templates/templates.router.ts(1 hunks)apps/api/src/template/services/template-fetcher/template-fetcher.service.ts(1 hunks)apps/api/src/template/services/template-gallery/template-gallery.service.ts(1 hunks)apps/api/src/template/services/template-processor/template-processor.service.ts(1 hunks)apps/api/src/template/types/template.ts(1 hunks)apps/api/src/utils/json.ts(1 hunks)apps/api/test/functional/templates.spec.ts(1 hunks)apps/api/test/mocks/templates/akash-network/awesome-akash/alpaca-cpp/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/awesome-akash/lunie-lite/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/awesome-akash/readme.md(1 hunks)apps/api/test/mocks/templates/akash-network/awesome-akash/ssh-ubuntu/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/cosmos-omnibus/akash/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/cosmos-omnibus/archway/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/cosmos-omnibus/index.json(1 hunks)apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/adguardhome-sync/index.json(1 hunks)apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/airsonic-advanced/index.json(1 hunks)apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/airsonic/index.json(1 hunks)apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/readme.md(1 hunks)apps/api/tsconfig.strict.json(1 hunks)
💤 Files with no reviewable changes (6)
- apps/api/src/routes/v1/index.ts
- apps/api/src/routes/v1/templates/list.ts
- apps/api/src/routes/v1/templates/list-full.ts
- apps/api/src/routes/v1/templates/byId.ts
- apps/api/src/services/external/templateReposService.ts
- apps/api/src/services/external/templatesCollector.ts
✅ Files skipped from review due to trivial changes (6)
- apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/airsonic-advanced/index.json
- apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/adguardhome-sync/index.json
- apps/api/test/mocks/templates/akash-network/cosmos-omnibus/archway/index.json
- apps/api/src/template/routes/templates/templates.router.ts
- apps/api/src/template/services/template-processor/template-processor.service.ts
- apps/api/src/template/http-schemas/template.schema.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- apps/api/src/app.ts
- apps/api/src/template/index.ts
- apps/api/tsconfig.strict.json
- apps/api/src/template/routes/index.ts
- apps/api/test/mocks/templates/akash-network/awesome-akash/lunie-lite/index.json
- apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/airsonic/index.json
- apps/api/test/mocks/templates/akash-network/cosmos-omnibus/index.json
- apps/api/test/mocks/templates/akash-network/cosmos-omnibus/akash/index.json
- apps/api/test/mocks/templates/akash-network/awesome-akash/ssh-ubuntu/index.json
- apps/api/src/template/controllers/template/template.controller.ts
- apps/api/test/mocks/templates/akash-network/awesome-akash/alpaca-cpp/index.json
- apps/api/src/template/types/template.ts
- apps/api/test/functional/templates.spec.ts
- apps/api/src/template/services/template-gallery/template-gallery.service.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/api/scripts/buildAkashTemplatesCache.ts (2)
apps/api/src/services/external/templates-collector.ts (1)
getTemplateGallery(7-10)apps/api/src/template/services/template-gallery/template-gallery.service.ts (1)
getTemplateGallery(23-58)
apps/api/src/services/external/templates-collector.ts (1)
apps/api/src/template/services/template-gallery/template-gallery.service.ts (1)
getTemplateGallery(23-58)
🪛 LanguageTool
apps/api/test/mocks/templates/akash-network/awesome-akash/readme.md
[grammar] ~5-~5: This phrase is duplicated. You should probably use “Cosmos SDK” only once.
Context: ... --> ### Official - Lunie Wallet for Cosmos SDK - [Cosmos SDK Node](https://github.com/ovrclk/akash-o...
(PHRASE_REPETITION)
apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/readme.md
[grammar] ~3-~3: Possible subject-verb agreement error detected.
Context: ...nverted to akash compliant YAML files. This make it easy for new users to deploy more ap...
(THIS_THAT_AGR)
[duplication] ~12-~12: Possible typo: you repeated a word.
Context: ...AdGuardHome Sync - Airsonic - Airsonic Advanced
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-api-build
- GitHub Check: validate-api
🔇 Additional comments (4)
apps/api/test/mocks/templates/akash-network/awesome-akash/readme.md (3)
1-2: Well-structured Mock Fixture Header
The main heading and the<!-- omit in toc -->directive are correctly applied to keep the TOC focused.
3-7: Verify Official Section Links
Please ensure that each directory referenced under the “Official” section (lunie-lite,akash-on-akash,ssh-ubuntu) exists in the mock path and includes areadme.md(or equivalent entry point) for the tests to consume.#!/bin/bash # Verify existence of official mock directories and their README files for dir in lunie-lite akash-on-akash ssh-ubuntu; do path="apps/api/test/mocks/templates/akash-network/awesome-akash/$dir" if [ ! -d "$path" ]; then echo "ERROR: Missing directory: $dir" exit 1 fi if [ ! -f "$path/readme.md" ]; then echo "ERROR: Missing readme.md in $dir" exit 1 fi done echo "All official mock directories verified"
9-11: To confirm whether the mock’s README file exists under a different name or casing, let’s search for any Markdown files in that directory:#!/bin/bash # List any Markdown files (case-insensitive) in alpaca-cpp mock directory dir="apps/api/test/mocks/templates/akash-network/awesome-akash/alpaca-cpp" echo "Searching for Markdown files in $dir:" find "$dir" -maxdepth 1 -type f -iname '*readme*.md' -printThis will reveal if the file is named differently (e.g.,
README.md) or missing entirely.apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/readme.md (1)
11-13: Duplicated bullet?Ensure the TOC list does not accidentally duplicate items (static analysis flagged a potential repeat).
| export const safeParseJson = <T>(json: string, defaultValue?: T): T | null => { | ||
| try { | ||
| return JSON.parse(json) as T; | ||
| } catch (error) { | ||
| return (defaultValue as T) ?? null; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Catch-all swallows non-JSON errors and hides useful diagnostics
Catching every thrown error (line 4) means that issues unrelated to JSON syntax—e.g., a RangeError caused by other code injected into the same try–block in the future—will be silently ignored and replaced by a default. Narrow the handler to SyntaxError (the only error JSON.parse should raise) and re-throw anything else so genuine bugs surface.
export const safeParseJson = <T>(json: string, defaultValue?: T): T | null => {
try {
return JSON.parse(json) as T;
} catch (error) {
- return (defaultValue as T) ?? null;
+ if (error instanceof SyntaxError) {
+ return defaultValue ?? null;
+ }
+ throw error; // preserve unexpected failures
}
};📝 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.
| export const safeParseJson = <T>(json: string, defaultValue?: T): T | null => { | |
| try { | |
| return JSON.parse(json) as T; | |
| } catch (error) { | |
| return (defaultValue as T) ?? null; | |
| } | |
| }; | |
| export const safeParseJson = <T>(json: string, defaultValue?: T): T | null => { | |
| try { | |
| return JSON.parse(json) as T; | |
| } catch (error) { | |
| if (error instanceof SyntaxError) { | |
| return defaultValue ?? null; | |
| } | |
| throw error; // preserve unexpected failures | |
| } | |
| }; |
🤖 Prompt for AI Agents
In apps/api/src/utils/json.ts around lines 1 to 7, the catch block currently
catches all errors, which can hide non-JSON parsing issues. Modify the catch
block to only handle SyntaxError exceptions from JSON.parse, and re-throw any
other error types to avoid masking genuine bugs.
apps/api/src/template/services/template-fetcher/template-fetcher.service.ts
Outdated
Show resolved
Hide resolved
apps/api/src/template/services/template-fetcher/template-fetcher.service.ts
Show resolved
Hide resolved
| constructor(private readonly templateProcessor: TemplateProcessorService) { | ||
| const GITHUB_PAT = env.GITHUB_PAT; | ||
| if (!GITHUB_PAT) throw new Error("Cannot fetch templates without GitHub PAT"); | ||
|
|
||
| this._octokit = getOctokit(GITHUB_PAT); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inject Octokit instead of constructing internally
Hard-wiring the PAT lookup and getOctokit() call inside the constructor couples the class to environment configuration and makes unit-testing hard (cannot easily stub the GitHub client).
Prefer receiving an Octokit instance (or a thin wrapper) via DI and fail fast if it is not registered.
- constructor(private readonly templateProcessor: TemplateProcessorService) {
- const GITHUB_PAT = env.GITHUB_PAT;
- if (!GITHUB_PAT) throw new Error("Cannot fetch templates without GitHub PAT");
-
- this._octokit = getOctokit(GITHUB_PAT);
- }
+ constructor(
+ private readonly templateProcessor: TemplateProcessorService,
+ @inject("Octokit") private readonly _octokit: Octokit
+ ) {}This also removes the need for the lazy octokit getter below.
📝 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.
| constructor(private readonly templateProcessor: TemplateProcessorService) { | |
| const GITHUB_PAT = env.GITHUB_PAT; | |
| if (!GITHUB_PAT) throw new Error("Cannot fetch templates without GitHub PAT"); | |
| this._octokit = getOctokit(GITHUB_PAT); | |
| } | |
| constructor( | |
| private readonly templateProcessor: TemplateProcessorService, | |
| @inject("Octokit") private readonly _octokit: Octokit | |
| ) {} |
🤖 Prompt for AI Agents
In apps/api/src/template/services/template-fetcher/template-fetcher.service.ts
around lines 36 to 41, the constructor currently creates an Octokit instance
internally using a PAT from the environment, which couples the class to
environment config and hinders testing. Refactor the constructor to accept an
Octokit instance (or a wrapper) via dependency injection instead of creating it
inside. Remove the internal PAT lookup and getOctokit call, and eliminate the
lazy octokit getter since the instance will be provided externally. Add a check
to fail fast if the injected Octokit instance is missing.
apps/api/src/template/services/template-fetcher/template-fetcher.service.ts
Show resolved
Hide resolved
apps/api/src/template/services/template-gallery/template-gallery.service.ts
Outdated
Show resolved
Hide resolved
|
|
||
| env.GITHUB_PAT = "ghp_1234567890"; | ||
|
|
||
| jest.mock("@octokit/rest", () => { |
There was a problem hiding this comment.
suggestion(non-blocking): avoid jest.mock usage in DI based systems. If you still need to usejest.mock in DI system, it means that some service doesn't properly encapsulate its dependencies
There was a problem hiding this comment.
My intention was to avoid real Octokit requests here.
Actual content on GitHub is subject to change for which it is hard to write expect lines. Also it is a rate limited API, and tests are eating that capacity quickly. I think faking an external resource is ok, isn't it? Can you suggest another way @stalniy?
There was a problem hiding this comment.
faking external resource is the right way to test the service. The actual issue is how we do it. jest.mock is a hacky way to do it. You can provide a mock for github service via DI container.
To simplify its definition, you can pass it as an optional dependency with default value which equal to Octocit instance. You can take a look at this example with feature flags -> https://github.com/akash-network/console/blob/main/apps/api/src/core/services/feature-flags/feature-flags.service.ts#L19
There was a problem hiding this comment.
Moved the whole logic outside of tsyringe world so cache generation still works in build time with minimal config. But then it's either Octokit itself or the getOctokit function we should mock and the mock would be the same in both cases (a fake Octokit instance we currently have in the branch). I think what we have now is what we can achieve here, or do I misunderstand and can it still be done in a better way?
jzsfkzm
force-pushed
the
refactor/1273-templates
branch
from
7d29a35 to
717d136
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
apps/api/src/template/services/template-fetcher/template-fetcher.service.ts (4)
20-20: Address the hard-coded branch name issueThe hard-coded
"master"branch names will cause failures for repositories that have switched to"main"as their default branch. This matches a previous review comment about this exact issue.Consider making the branch name configurable or implement fallback logic:
export const REPOSITORIES = { "awesome-akash": { repoOwner: "akash-network", repoName: "awesome-akash", - mainBranch: "master" + mainBranch: process.env.AWESOME_AKASH_BRANCH ?? "master" }, "cosmos-omnibus": { repoOwner: "akash-network", repoName: "cosmos-omnibus", - mainBranch: "master" + mainBranch: process.env.COSMOS_OMNIBUS_BRANCH ?? "master" }, "akash-linuxserver": { repoOwner: "cryptoandcoffee", repoName: "akash-linuxserver", mainBranch: "main" } };Alternatively, implement a helper method that tries both branches and falls back gracefully.
Also applies to: 25-25, 30-30
39-44: Refactor constructor to use dependency injectionThe constructor creates an Octokit instance internally, which couples the class to environment configuration and makes testing difficult. This matches a previous review comment about this exact issue.
-constructor(private readonly templateProcessor: TemplateProcessorService) { - const GITHUB_PAT = env.GITHUB_PAT; - if (!GITHUB_PAT) throw new Error("Cannot fetch templates without GitHub PAT"); - - this._octokit = getOctokit(GITHUB_PAT); -} +constructor( + private readonly templateProcessor: TemplateProcessorService, + @inject("Octokit") private readonly _octokit: Octokit +) {}
75-93: Handle large file redirects and improve type safetyThe current implementation doesn't handle GitHub's 302 redirects for files larger than 1MB, which will result in "[object Object]" being returned instead of file content. This matches a previous review comment about this exact issue.
-return String(response.data); +if (typeof response.data === "string") return response.data; +if ("url" in response.data) { + const redirected = await fetch(response.data.url); + if (!redirected.ok) { + throw new Error(`Failed to fetch redirected content ${response.data.url}`); + } + return await redirected.text(); +} +throw new Error("Unsupported GitHub content response shape");
125-133: Add status validation to prevent silent failuresThe fetch response is used without checking if the HTTP status is successful, which can lead to silent failures when GitHub returns 404. This matches a previous review comment about this exact issue.
private async findFileContentAsync(filename: string | string[], fileList: GithubDirectoryItem[]): Promise<string | null> { const filenames = typeof filename === "string" ? [filename] : filename; const fileDef = fileList.find(f => filenames.some(x => x.toLowerCase() === f.name.toLowerCase())); if (!fileDef) return null; const response = await fetch(fileDef.download_url); + if (!response.ok) return null; return response.text(); }
🧹 Nitpick comments (5)
apps/api/src/template/services/template-fetcher/template-fetcher.service.ts (3)
158-158: Remove debug loggingDebug console.log statement should be removed or replaced with proper logging.
-console.log(templateSource.name);
232-257: Complex regex parsing could be more robustThe current regex-based parsing of README files is fragile and may miss edge cases. Consider using a markdown parser for more reliable parsing.
This parsing logic could benefit from a dedicated markdown parsing library to handle edge cases and improve maintainability:
// Consider using a library like 'remark' or 'markdown-it' import { remark } from 'remark'; import remarkWiki from 'remark-wiki-link';
287-288: Replace console.log/console.error with proper loggingDirect console logging should be replaced with a proper logging service for better observability.
-console.log("Could not fetch chain for", templateSource.path); -console.error(err); +// Use proper logging service instead +this.logger.warn(`Could not fetch chain for ${templateSource.path}`, err);apps/api/test/mocks/templates/akash-network/awesome-akash/readme.md (1)
3-3: Consider using proper heading hierarchyThe heading structure jumps from h1 to h3, which may cause accessibility issues. However, since this is mock test data, it's acceptable as-is.
-### Official +## OfficialAnd similarly for other h3 headings. But since this is test mock data, the current structure is fine for testing purposes.
apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/readme.md (1)
3-3: Fix grammar in descriptionThere's a subject-verb agreement error in the description.
-akash-linuxserver is a mirror of linuxserver docker images that have been converted to akash compliant YAML files. This make it easy for new users to deploy more applications on the [Akash](https://akash.network) network. +akash-linuxserver is a mirror of linuxserver docker images that have been converted to akash compliant YAML files. This makes it easy for new users to deploy more applications on the [Akash](https://akash.network) network.Since this is mock test data, the grammatical error doesn't affect functionality, but it's good practice to maintain quality even in test fixtures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
apps/api/scripts/buildAkashTemplatesCache.ts(2 hunks)apps/api/src/app.ts(2 hunks)apps/api/src/routes/v1/index.ts(0 hunks)apps/api/src/routes/v1/templates/byId.ts(0 hunks)apps/api/src/routes/v1/templates/list-full.ts(0 hunks)apps/api/src/routes/v1/templates/list.ts(0 hunks)apps/api/src/services/external/templateReposService.ts(0 hunks)apps/api/src/services/external/templates-collector.ts(1 hunks)apps/api/src/services/external/templatesCollector.ts(0 hunks)apps/api/src/template/controllers/template/template.controller.ts(1 hunks)apps/api/src/template/http-schemas/template.schema.ts(1 hunks)apps/api/src/template/index.ts(1 hunks)apps/api/src/template/routes/index.ts(1 hunks)apps/api/src/template/routes/templates/templates.router.ts(1 hunks)apps/api/src/template/services/template-fetcher/template-fetcher.service.ts(1 hunks)apps/api/src/template/services/template-gallery/template-gallery.service.ts(1 hunks)apps/api/src/template/services/template-processor/template-processor.service.ts(1 hunks)apps/api/src/template/types/template.ts(1 hunks)apps/api/src/utils/json.ts(1 hunks)apps/api/test/functional/templates.spec.ts(1 hunks)apps/api/test/mocks/templates/akash-network/awesome-akash/alpaca-cpp/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/awesome-akash/lunie-lite/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/awesome-akash/readme.md(1 hunks)apps/api/test/mocks/templates/akash-network/awesome-akash/ssh-ubuntu/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/cosmos-omnibus/akash/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/cosmos-omnibus/archway/index.json(1 hunks)apps/api/test/mocks/templates/akash-network/cosmos-omnibus/index.json(1 hunks)apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/adguardhome-sync/index.json(1 hunks)apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/airsonic-advanced/index.json(1 hunks)apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/airsonic/index.json(1 hunks)apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/readme.md(1 hunks)apps/api/tsconfig.strict.json(1 hunks)
💤 Files with no reviewable changes (6)
- apps/api/src/routes/v1/templates/list.ts
- apps/api/src/routes/v1/index.ts
- apps/api/src/routes/v1/templates/list-full.ts
- apps/api/src/routes/v1/templates/byId.ts
- apps/api/src/services/external/templateReposService.ts
- apps/api/src/services/external/templatesCollector.ts
✅ Files skipped from review due to trivial changes (8)
- apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/adguardhome-sync/index.json
- apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/airsonic-advanced/index.json
- apps/api/src/template/controllers/template/template.controller.ts
- apps/api/test/mocks/templates/akash-network/awesome-akash/lunie-lite/index.json
- apps/api/test/mocks/templates/akash-network/awesome-akash/alpaca-cpp/index.json
- apps/api/src/template/routes/templates/templates.router.ts
- apps/api/src/template/services/template-processor/template-processor.service.ts
- apps/api/src/template/http-schemas/template.schema.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- apps/api/src/template/index.ts
- apps/api/tsconfig.strict.json
- apps/api/src/template/routes/index.ts
- apps/api/test/mocks/templates/akash-network/cosmos-omnibus/index.json
- apps/api/src/utils/json.ts
- apps/api/src/app.ts
- apps/api/scripts/buildAkashTemplatesCache.ts
- apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/airsonic/index.json
- apps/api/src/services/external/templates-collector.ts
- apps/api/test/mocks/templates/akash-network/cosmos-omnibus/archway/index.json
- apps/api/test/mocks/templates/akash-network/awesome-akash/ssh-ubuntu/index.json
- apps/api/test/mocks/templates/akash-network/cosmos-omnibus/akash/index.json
- apps/api/test/functional/templates.spec.ts
- apps/api/src/template/types/template.ts
- apps/api/src/template/services/template-gallery/template-gallery.service.ts
🧰 Additional context used
🪛 LanguageTool
apps/api/test/mocks/templates/akash-network/awesome-akash/readme.md
[grammar] ~5-~5: This phrase is duplicated. You should probably use “Cosmos SDK” only once.
Context: ... --> ### Official - Lunie Wallet for Cosmos SDK - [Cosmos SDK Node](https://github.com/ovrclk/akash-o...
(PHRASE_REPETITION)
apps/api/test/mocks/templates/cryptoandcoffee/akash-linuxserver/readme.md
[grammar] ~3-~3: Possible subject-verb agreement error detected.
Context: ...nverted to akash compliant YAML files. This make it easy for new users to deploy more ap...
(THIS_THAT_AGR)
[duplication] ~12-~12: Possible typo: you repeated a word.
Context: ...AdGuardHome Sync - Airsonic - Airsonic Advanced
(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
apps/api/test/mocks/templates/akash-network/awesome-akash/readme.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-api-build
- GitHub Check: validate-api
jzsfkzm
force-pushed
the
refactor/1273-templates
branch
2 times, most recently
from
0c2842e to
ad3b117
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/api/src/services/external/templates/template-processor.service.ts (1)
98-98: Persistent storage detection is overly simplistic and may miss valid configurations.The current string matching approach may miss valid YAML configurations with different spacing, indentation, or formatting.
Consider using a proper YAML parser for more reliable detection:
- persistentStorageEnabled: !!deploy && (deploy.includes("persistent: true") || deploy.includes("persistent:true")), + persistentStorageEnabled: this.detectPersistentStorage(deploy),Add a dedicated method:
private detectPersistentStorage(deploy: string): boolean { if (!deploy) return false; // More comprehensive pattern matching const patterns = [ /persistent:\s*true/i, /persistent:\s*"true"/i, /persistent:\s*'true'/i ]; return patterns.some(pattern => pattern.test(deploy)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
apps/api/scripts/buildAkashTemplatesCache.ts(2 hunks)apps/api/src/app.ts(2 hunks)apps/api/src/routes/v1/index.ts(1 hunks)apps/api/src/routes/v1/templates/byId.ts(0 hunks)apps/api/src/routes/v1/templates/list-full.ts(0 hunks)apps/api/src/routes/v1/templates/list.ts(0 hunks)apps/api/src/services/external/templateReposService.ts(0 hunks)apps/api/src/services/external/templates/template-fetcher.service.ts(1 hunks)apps/api/src/services/external/templates/template-gallery.service.ts(1 hunks)apps/api/src/services/external/templates/template-processor.service.ts(1 hunks)apps/api/src/services/external/templatesCollector.ts(0 hunks)apps/api/src/template/controllers/template/template.controller.ts(1 hunks)apps/api/src/template/http-schemas/template.schema.ts(1 hunks)apps/api/src/template/index.ts(1 hunks)apps/api/src/template/routes/index.ts(1 hunks)apps/api/src/template/routes/templates/templates.router.ts(1 hunks)apps/api/src/template/types/template.ts(1 hunks)apps/api/src/utils/json.ts(1 hunks)apps/api/test/functional/template-cache.spec.ts(1 hunks)apps/api/test/functional/templates.spec.ts(1 hunks)apps/api/tsconfig.strict.json(1 hunks)
💤 Files with no reviewable changes (5)
- apps/api/src/routes/v1/templates/list-full.ts
- apps/api/src/routes/v1/templates/list.ts
- apps/api/src/routes/v1/templates/byId.ts
- apps/api/src/services/external/templatesCollector.ts
- apps/api/src/services/external/templateReposService.ts
✅ Files skipped from review due to trivial changes (2)
- apps/api/src/routes/v1/index.ts
- apps/api/src/services/external/templates/template-fetcher.service.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- apps/api/src/app.ts
- apps/api/src/template/index.ts
- apps/api/tsconfig.strict.json
- apps/api/src/template/routes/index.ts
- apps/api/test/functional/template-cache.spec.ts
- apps/api/scripts/buildAkashTemplatesCache.ts
- apps/api/src/utils/json.ts
- apps/api/test/functional/templates.spec.ts
- apps/api/src/template/controllers/template/template.controller.ts
- apps/api/src/services/external/templates/template-gallery.service.ts
- apps/api/src/template/types/template.ts
- apps/api/src/template/routes/templates/templates.router.ts
- apps/api/src/template/http-schemas/template.schema.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/api/src/services/external/templates/template-processor.service.ts (3)
apps/api/src/template/types/template.ts (4)
Category(11-16)TemplateSource(1-9)Template(25-37)TemplateConfig(20-23)apps/api/src/utils/json.ts (1)
safeParseJson(1-7)apps/api/src/utils/templateReposLogos.ts (1)
getLogoFromPath(1-3)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (2)
apps/api/src/services/external/templates/template-processor.service.ts (2)
78-114: LGTM! Well-structured template processing logic.The
processTemplatemethod demonstrates good practices:
- Early validation with null checks
- Proper fallback handling for optional fields
- Sensible default configurations
- Good separation of concerns with helper methods
The method effectively consolidates all template processing logic into a single, well-organized function.
55-69: Link replacement logic may cause unintended text substitutions.The current string replacement approach using
split().join()could replace unintended occurrences of the same URL pattern elsewhere in the markdown.Replace the text replacement logic with a more precise approach:
- newMarkdown = newMarkdown.split("(" + originalUrl + ")").join("(" + absoluteUrl + ")"); + newMarkdown = newMarkdown.replace( + new RegExp("\\(" + originalUrl.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + "\\)", 'g'), + "(" + absoluteUrl + ")" + );Likely an incorrect or invalid review comment.
| removeComments(markdown: string): string { | ||
| return markdown.replace(/<!--.+-->/g, ""); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
HTML comment removal regex doesn't handle multi-line comments.
The current regex /<!--.+-->/g won't match HTML comments that span multiple lines because . doesn't match newline characters by default.
Fix the regex to handle multi-line comments:
- return markdown.replace(/<!--.+-->/g, "");
+ return markdown.replace(/<!--[\s\S]*?-->/g, "");📝 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.
| removeComments(markdown: string): string { | |
| return markdown.replace(/<!--.+-->/g, ""); | |
| } | |
| removeComments(markdown: string): string { | |
| return markdown.replace(/<!--[\s\S]*?-->/g, ""); | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/services/external/templates/template-processor.service.ts around
lines 74 to 76, the regex used to remove HTML comments does not handle
multi-line comments because the dot (.) does not match newline characters.
Update the regex to use a pattern that matches across multiple lines, such as
using a non-greedy match with [\s\S]* or enabling the dotall flag if supported,
to ensure all multi-line HTML comments are properly removed.
| getTemplateSummary(readme: string): string | null { | ||
| if (!readme) return null; | ||
|
|
||
| const markdown = readme | ||
| .replace(/!\[.*\]\(.+\)\n*/g, "") // Remove images | ||
| .replace(/^#+ .*\n+/g, ""); // Remove first header | ||
|
|
||
| const readmeTxt = markdownToTxt(markdown).trim(); | ||
| const maxLength = 200; | ||
| return readmeTxt.length > maxLength ? readmeTxt.substring(0, maxLength - 3).trim() + "..." : readmeTxt; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for markdown processing methods.
The methods assume string input but don't validate the type, which could cause runtime errors if non-string values are passed.
Add type validation:
getTemplateSummary(readme: string): string | null {
- if (!readme) return null;
+ if (!readme || typeof readme !== 'string') return null;Apply similar validation to other methods that process string inputs.
📝 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.
| getTemplateSummary(readme: string): string | null { | |
| if (!readme) return null; | |
| const markdown = readme | |
| .replace(/!\[.*\]\(.+\)\n*/g, "") // Remove images | |
| .replace(/^#+ .*\n+/g, ""); // Remove first header | |
| const readmeTxt = markdownToTxt(markdown).trim(); | |
| const maxLength = 200; | |
| return readmeTxt.length > maxLength ? readmeTxt.substring(0, maxLength - 3).trim() + "..." : readmeTxt; | |
| } | |
| getTemplateSummary(readme: string): string | null { | |
| if (!readme || typeof readme !== 'string') return null; | |
| const markdown = readme | |
| .replace(/!\[.*\]\(.+\)\n*/g, "") // Remove images | |
| .replace(/^#+ .*\n+/g, ""); // Remove first header | |
| const readmeTxt = markdownToTxt(markdown).trim(); | |
| const maxLength = 200; | |
| return readmeTxt.length > maxLength | |
| ? readmeTxt.substring(0, maxLength - 3).trim() + "..." | |
| : readmeTxt; | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/services/external/templates/template-processor.service.ts around
lines 22 to 32, the getTemplateSummary method assumes the input readme is always
a string but lacks type validation, which can cause runtime errors if a
non-string is passed. Add a type check at the start of the method to verify that
readme is a string; if not, return null or handle it appropriately. Apply
similar type validation to other methods in this service that process string
inputs to ensure robustness.
| mergeTemplateCategories(...categories: Category[][]): Category[] { | ||
| const mergedCategories: Category[] = []; | ||
| for (const category of categories.flat()) { | ||
| const existingCategory = mergedCategories.find(c => c.title.toLowerCase() === category.title.toLowerCase()); | ||
| if (existingCategory) { | ||
| existingCategory.templates = (existingCategory.templates || []).concat(category.templates); | ||
| } else { | ||
| mergedCategories.push(JSON.parse(JSON.stringify(category))); | ||
| } | ||
| } | ||
| return mergedCategories; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inefficient deep cloning approach may cause performance and reliability issues.
The JSON.parse(JSON.stringify()) approach for deep cloning has several drawbacks:
- Performance overhead for large objects
- Loses functions, undefined values, and symbols
- Fails with circular references
- Doesn't preserve prototype chain
Consider using a proper deep cloning utility or restructure to avoid deep cloning:
- mergedCategories.push(JSON.parse(JSON.stringify(category)));
+ mergedCategories.push({
+ ...category,
+ templates: [...category.templates],
+ templateSources: [...category.templateSources]
+ });🤖 Prompt for AI Agents
In apps/api/src/services/external/templates/template-processor.service.ts around
lines 9 to 20, replace the JSON.parse(JSON.stringify(category)) deep cloning
with a more reliable deep cloning method such as using a utility like lodash's
cloneDeep or a custom deep clone function. This will improve performance, handle
circular references, preserve functions and prototype chains, and avoid losing
undefined or symbol values.
ad3b117 to
2497296
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
apps/api/test/functional/templates.spec.ts (1)
17-37: Avoidjest.mockusage in DI based systems.This issue was already raised in a previous review. Consider providing a mock for the GitHub service via DI container instead of using
jest.mock.
🧹 Nitpick comments (4)
apps/api/src/services/external/templates/template-gallery.service.ts (2)
16-17: Consider using Map for better type safety.Using a
Mapinstead ofRecordfor dynamic key-value stores provides better type safety and performance.- private generatingTasks: Record<string, Promise<Category[]> | null> = {}; + private generatingTasks = new Map<string, Promise<Category[]> | null>();Then update the usage:
- } else if (this.generatingTasks[cacheFilePath]) { + } else if (this.generatingTasks.get(cacheFilePath)) { console.log("Waiting on existing task for", repoOwner, repoName); - return await this.generatingTasks[cacheFilePath]; + return await this.generatingTasks.get(cacheFilePath)!; } else { console.log("No cache found for", repoOwner, repoName, "generating..."); - this.generatingTasks[cacheFilePath] = fetchTemplates(repoVersion); - const categories = await this.generatingTasks[cacheFilePath]; - this.generatingTasks[cacheFilePath] = null; + this.generatingTasks.set(cacheFilePath, fetchTemplates(repoVersion)); + const categories = await this.generatingTasks.get(cacheFilePath)!; + this.generatingTasks.delete(cacheFilePath);
87-90: Simplify file existence check.The current pattern is less readable than alternatives.
- const cacheFileExists = await fs.promises.access(cacheFilePath, fs.constants.R_OK).then( - () => true, - () => false - ); + const cacheFileExists = fs.existsSync(cacheFilePath);Or if you prefer async:
- const cacheFileExists = await fs.promises.access(cacheFilePath, fs.constants.R_OK).then( - () => true, - () => false - ); + let cacheFileExists = false; + try { + await fs.promises.access(cacheFilePath, fs.constants.R_OK); + cacheFileExists = true; + } catch { + // File doesn't exist + }apps/api/src/services/external/templates/template-fetcher.service.ts (2)
107-111: Avoid type assertion by improving response typing.The type assertion could be avoided with better typing of the Octokit response.
- if (!Array.isArray(response.data)) { - throw new Error(`Failed to fetch directory content from ${owner}/${repo}/${path}`); - } - - return response.data as GithubDirectoryItem[]; + const data = response.data; + if (!Array.isArray(data)) { + throw new Error(`Failed to fetch directory content from ${owner}/${repo}/${path}`); + } + + return data;
329-340: Preserve error context in concurrent operations.When aggregating errors from concurrent operations, include context about which operation failed.
if (errors.length > 0) { - throw new Error(errors.map(e => e.message).join("\n")); + const errorDetails = errors.map((e, i) => `Operation ${i}: ${e.message}`).join("\n"); + throw new Error(`Multiple operations failed:\n${errorDetails}`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
apps/api/scripts/buildAkashTemplatesCache.ts(2 hunks)apps/api/src/app.ts(2 hunks)apps/api/src/routes/v1/index.ts(1 hunks)apps/api/src/routes/v1/templates/byId.ts(0 hunks)apps/api/src/routes/v1/templates/list-full.ts(0 hunks)apps/api/src/routes/v1/templates/list.ts(0 hunks)apps/api/src/services/external/templateReposService.ts(0 hunks)apps/api/src/services/external/templates/template-fetcher.service.ts(1 hunks)apps/api/src/services/external/templates/template-gallery.service.ts(1 hunks)apps/api/src/services/external/templates/template-processor.service.ts(1 hunks)apps/api/src/services/external/templatesCollector.ts(0 hunks)apps/api/src/template/controllers/template/template.controller.ts(1 hunks)apps/api/src/template/http-schemas/template.schema.ts(1 hunks)apps/api/src/template/index.ts(1 hunks)apps/api/src/template/routes/index.ts(1 hunks)apps/api/src/template/routes/templates/templates.router.ts(1 hunks)apps/api/src/template/types/template.ts(1 hunks)apps/api/src/utils/json.ts(1 hunks)apps/api/test/functional/template-cache.spec.ts(1 hunks)apps/api/test/functional/templates.spec.ts(1 hunks)apps/api/tsconfig.strict.json(1 hunks)
💤 Files with no reviewable changes (5)
- apps/api/src/routes/v1/templates/list.ts
- apps/api/src/routes/v1/templates/list-full.ts
- apps/api/src/services/external/templateReposService.ts
- apps/api/src/routes/v1/templates/byId.ts
- apps/api/src/services/external/templatesCollector.ts
✅ Files skipped from review due to trivial changes (1)
- apps/api/src/routes/v1/index.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/api/src/template/index.ts
- apps/api/src/template/routes/index.ts
- apps/api/tsconfig.strict.json
- apps/api/scripts/buildAkashTemplatesCache.ts
- apps/api/test/functional/template-cache.spec.ts
- apps/api/src/app.ts
- apps/api/src/template/controllers/template/template.controller.ts
- apps/api/src/utils/json.ts
- apps/api/src/template/routes/templates/templates.router.ts
- apps/api/src/services/external/templates/template-processor.service.ts
- apps/api/src/template/http-schemas/template.schema.ts
- apps/api/src/template/types/template.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.spec.{ts,tsx}`: Use `setup` function instead of `beforeEach` in test files...
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
📄 Source: CodeRabbit Inference Engine (.cursor/rules/setup-instead-of-before-each.mdc)
List of files the instruction was applied to:
apps/api/test/functional/templates.spec.ts
🧠 Learnings (1)
apps/api/test/functional/templates.spec.ts (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: stalniy
PR: akash-network/console#1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
🧬 Code Graph Analysis (1)
apps/api/test/functional/templates.spec.ts (2)
apps/api/src/template/http-schemas/template.schema.ts (3)
GetTemplatesFullResponse(25-25)GetTemplatesListResponse(30-30)GetTemplateByIdResponse(42-42)apps/deploy-web/tests/fixture/context-with-extension.ts (1)
expect(77-77)
🪛 GitHub Check: validate / validate-app
apps/api/test/functional/templates.spec.ts
[failure] 6-6:
All imports in the declaration are only used as types. Use import type
[failure] 1-1:
Run autofix to sort these imports!
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test-build
🔇 Additional comments (1)
apps/api/src/services/external/templates/template-gallery.service.ts (1)
54-62: Good error handling with fallback mechanism.The fallback to
lastServedDataensures API availability even when external services fail. This is a good resilience pattern.
| if (cacheFileExists) { | ||
| console.log("Returning cached templates from", cacheFilePath); | ||
| const fileContent = await fs.promises.readFile(cacheFilePath, "utf8"); | ||
| return JSON.parse(fileContent); |
There was a problem hiding this comment.
Add error handling for JSON.parse.
If the cached file is corrupted or contains invalid JSON, JSON.parse will throw an unhandled exception.
if (cacheFileExists) {
console.log("Returning cached templates from", cacheFilePath);
- const fileContent = await fs.promises.readFile(cacheFilePath, "utf8");
- return JSON.parse(fileContent);
+ try {
+ const fileContent = await fs.promises.readFile(cacheFilePath, "utf8");
+ return JSON.parse(fileContent);
+ } catch (error) {
+ console.error(`Failed to parse cached file ${cacheFilePath}:`, error);
+ // Delete corrupted cache file
+ await fs.promises.unlink(cacheFilePath).catch(() => {});
+ // Fall through to regenerate
+ }
} else if (this.generatingTasks[cacheFilePath]) {🤖 Prompt for AI Agents
In apps/api/src/services/external/templates/template-gallery.service.ts around
lines 92 to 95, the code uses JSON.parse on the cached file content without
error handling, which can cause unhandled exceptions if the JSON is invalid.
Wrap the JSON.parse call in a try-catch block to catch any parsing errors, log
or handle the error appropriately, and decide whether to return a fallback value
or rethrow the error to prevent the application from crashing.
apps/api/src/services/external/templates/template-fetcher.service.ts
Outdated
Show resolved
Hide resolved
jzsfkzm
force-pushed
the
refactor/1273-templates
branch
from
2497296 to
f98ddd9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/api/src/services/external/templates/template-fetcher.service.ts (3)
33-34: Fix type forgithubRequestsRemaining.The GitHub API returns the rate limit remaining as a number, not a string.
- private githubRequestsRemaining: string | null | undefined = null; + private githubRequestsRemaining: number | null = null;Update the assignments accordingly:
- this.githubRequestsRemaining = response.headers["x-ratelimit-remaining"]; + this.githubRequestsRemaining = parseInt(response.headers["x-ratelimit-remaining"] as string, 10);
114-122: Avoid double type assertion.Using
as unknown asis a code smell that bypasses TypeScript's type checking.- return response.json() as unknown as GithubChainRegistryChainResponse; + return await response.json() as GithubChainRegistryChainResponse;Or better yet, add runtime validation:
- return response.json() as unknown as GithubChainRegistryChainResponse; + const data = await response.json(); + // Add validation here if needed + return data as GithubChainRegistryChainResponse;
144-146: Improve error handling.
- Throw Error objects instead of strings for better debugging
- Consider re-throwing critical errors instead of just logging warnings
if (templateSource.path.startsWith("http:") || templateSource.path.startsWith("https:")) { - throw "Absolute URL"; + throw new Error("Absolute URL not supported"); }} catch (err: any) { - console.warn(`Skipped ${templateSource.name} because of error: ${err.message || err}`); + const errorMessage = err instanceof Error ? err.message : String(err); + console.warn(`Skipped ${templateSource.name} because of error: ${errorMessage}`); + // Consider if some errors should be re-thrown + if (err instanceof TypeError || err instanceof ReferenceError) { + throw err; + } return null; }Also applies to: 161-163
🧹 Nitpick comments (3)
apps/api/src/services/external/templates/template-fetcher.service.ts (3)
231-246: Consider more robust parsing for README content.The regex patterns used to parse categories and templates from README files are fragile and may fail silently if the markdown format changes. Consider adding validation or using a proper markdown parser.
const categoryRegex = /### (.+)\n*([\w ]+)?\n*((?:- \[(?:.+)]\((?:.+)\)\n?)*)/gm; const templateRegex = /(- \[(.+)]\((.+)\)\n?)/gm; const categories: Category[] = []; const matches = data.matchAll(categoryRegex); + if (!matches || Array.from(matches).length === 0) { + console.warn(`No categories found in README at ${readmePath}`); + } for (const match of matches) { const title = match[1]; const description = match[2]; const templatesStr = match[3]; if (categories.some(x => x.title === title) || !templatesStr) continue; const templateSources: TemplateSource[] = []; const templateMatches = templatesStr.matchAll(templateRegex); + if (!templateMatches || Array.from(templateMatches).length === 0) { + console.warn(`No templates found for category "${title}"`); + } for (const templateMatch of templateMatches) { templateSources.push(templateSourceMapper(templateMatch[2], templateMatch[3])); }
280-289: Track failed chain registry fetches for monitoring.The error handling silently continues when chain registry data cannot be fetched. Consider tracking which templates failed for monitoring purposes.
+ const failedChains: string[] = []; await this.mapConcurrently( templateSources, async templateSource => { try { const chainData = await this.fetchChainRegistryData(templateSource.path); templateSource.name = chainData.pretty_name; templateSource.summary = chainData.description ?? templateSource.summary; templateSource.logoUrl = Object.values(chainData.logo_URIs ?? {})[0]; } catch (err) { console.log("Could not fetch chain for", templateSource.path); console.error(err); + failedChains.push(templateSource.path); } }, { concurrency: 5 } ); + + if (failedChains.length > 0) { + console.warn(`Failed to fetch chain registry data for ${failedChains.length} chains:`, failedChains); + }
335-337: Improve error aggregation for better debugging.Throwing all errors as a single concatenated string loses context about which operations failed. Consider creating a more structured error.
if (errors.length > 0) { - throw new Error(errors.map(e => e.message).join("\n")); + const errorDetails = errors.map((e, i) => ({ + index: e.index, + item: array[e.index], + error: e.message + })); + const error = new Error(`Failed to process ${errors.length} items`); + (error as any).details = errorDetails; + throw error; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
apps/api/scripts/buildAkashTemplatesCache.ts(2 hunks)apps/api/src/app.ts(2 hunks)apps/api/src/routes/v1/index.ts(1 hunks)apps/api/src/routes/v1/templates/byId.ts(0 hunks)apps/api/src/routes/v1/templates/list-full.ts(0 hunks)apps/api/src/routes/v1/templates/list.ts(0 hunks)apps/api/src/services/external/templateReposService.ts(0 hunks)apps/api/src/services/external/templates/template-fetcher.service.ts(1 hunks)apps/api/src/services/external/templates/template-gallery.service.ts(1 hunks)apps/api/src/services/external/templates/template-processor.service.ts(1 hunks)apps/api/src/services/external/templatesCollector.ts(0 hunks)apps/api/src/template/controllers/template/template.controller.ts(1 hunks)apps/api/src/template/http-schemas/template.schema.ts(1 hunks)apps/api/src/template/index.ts(1 hunks)apps/api/src/template/routes/index.ts(1 hunks)apps/api/src/template/routes/templates/templates.router.ts(1 hunks)apps/api/src/template/types/template.ts(1 hunks)apps/api/src/utils/json.ts(1 hunks)apps/api/test/functional/template-cache.spec.ts(1 hunks)apps/api/test/functional/templates.spec.ts(1 hunks)apps/api/tsconfig.strict.json(1 hunks)
💤 Files with no reviewable changes (5)
- apps/api/src/routes/v1/templates/list.ts
- apps/api/src/routes/v1/templates/list-full.ts
- apps/api/src/routes/v1/templates/byId.ts
- apps/api/src/services/external/templateReposService.ts
- apps/api/src/services/external/templatesCollector.ts
✅ Files skipped from review due to trivial changes (1)
- apps/api/src/routes/v1/index.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- apps/api/tsconfig.strict.json
- apps/api/src/app.ts
- apps/api/src/template/index.ts
- apps/api/scripts/buildAkashTemplatesCache.ts
- apps/api/src/utils/json.ts
- apps/api/src/template/routes/index.ts
- apps/api/test/functional/template-cache.spec.ts
- apps/api/test/functional/templates.spec.ts
- apps/api/src/template/controllers/template/template.controller.ts
- apps/api/src/template/routes/templates/templates.router.ts
- apps/api/src/template/http-schemas/template.schema.ts
- apps/api/src/template/types/template.ts
- apps/api/src/services/external/templates/template-processor.service.ts
- apps/api/src/services/external/templates/template-gallery.service.ts
🧰 Additional context used
🧠 Learnings (1)
apps/api/src/services/external/templates/template-fetcher.service.ts (1)
Learnt from: stalniy
PR: akash-network/console#1362
File: apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts:21-21
Timestamp: 2025-05-24T04:26:25.511Z
Learning: In TypeScript strict mode, double casts like `c as unknown as AppContext` may be necessary even when types are structurally compatible, as strict mode enforces more restrictive type compatibility rules than regular mode.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (1)
apps/api/src/services/external/templates/template-fetcher.service.ts (1)
32-341: Well-structured service with good separation of concerns.The service demonstrates good architectural patterns:
- Clear dependency injection
- Proper error handling boundaries
- Efficient concurrent processing with rate limiting
- Well-organized methods with single responsibilities
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/api/src/services/external/templates/template-gallery.service.ts (1)
92-95: Add error handling for JSON.parse.If the cached file is corrupted or contains invalid JSON,
JSON.parsewill throw an unhandled exception.if (cacheFileExists) { console.log("Returning cached templates from", cacheFilePath); - const fileContent = await fs.promises.readFile(cacheFilePath, "utf8"); - return JSON.parse(fileContent); + try { + const fileContent = await fs.promises.readFile(cacheFilePath, "utf8"); + return JSON.parse(fileContent); + } catch (error) { + console.error(`Failed to parse cached file ${cacheFilePath}:`, error); + // Delete corrupted cache file + await fs.promises.unlink(cacheFilePath).catch(() => {}); + // Fall through to regenerate + } } else if (this.generatingTasks[cacheFilePath]) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api/src/services/external/templates/template-fetcher.service.ts(1 hunks)apps/api/src/services/external/templates/template-gallery.service.ts(1 hunks)apps/api/test/functional/template-cache.spec.ts(1 hunks)apps/api/test/functional/templates.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/api/test/functional/template-cache.spec.ts
- apps/api/test/functional/templates.spec.ts
- apps/api/src/services/external/templates/template-fetcher.service.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (3)
apps/api/src/services/external/templates/template-gallery.service.ts (3)
15-25: LGTM! Clean dependency injection pattern.The constructor properly initializes dependencies and follows good dependency injection practices.
27-63: Well-structured main method with good error handling.The concurrent fetching strategy and fallback mechanism to last served data is well implemented. The error handling ensures service availability even when fetching fails.
65-76: Efficient template lookup implementation.The method correctly leverages the cached gallery and provides a clean interface for single template retrieval.
| await fs.promises.mkdir(path.dirname(cacheFilePath), { recursive: true }); | ||
| await fs.promises.writeFile(cacheFilePath, JSON.stringify(categories, null, 2)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for file system operations.
The mkdir and writeFile operations could fail, leaving the cache in an inconsistent state. If these operations fail after setting the generatingTasks entry to null, subsequent calls would unnecessarily regenerate the data.
- await fs.promises.mkdir(path.dirname(cacheFilePath), { recursive: true });
- await fs.promises.writeFile(cacheFilePath, JSON.stringify(categories, null, 2));
+ try {
+ await fs.promises.mkdir(path.dirname(cacheFilePath), { recursive: true });
+ await fs.promises.writeFile(cacheFilePath, JSON.stringify(categories, null, 2));
+ } catch (error) {
+ console.error(`Failed to cache templates to ${cacheFilePath}:`, error);
+ // Continue without caching - the data is still valid
+ }📝 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.
| await fs.promises.mkdir(path.dirname(cacheFilePath), { recursive: true }); | |
| await fs.promises.writeFile(cacheFilePath, JSON.stringify(categories, null, 2)); | |
| try { | |
| await fs.promises.mkdir(path.dirname(cacheFilePath), { recursive: true }); | |
| await fs.promises.writeFile(cacheFilePath, JSON.stringify(categories, null, 2)); | |
| } catch (error) { | |
| console.error(`Failed to cache templates to ${cacheFilePath}:`, error); | |
| // Continue without caching - the data is still valid | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/services/external/templates/template-gallery.service.ts around
lines 105 to 106, add error handling for the mkdir and writeFile file system
operations to prevent inconsistent cache states. Wrap these operations in a
try-catch block, and in case of an error, ensure the generatingTasks entry is
properly reset or handled to avoid unnecessary regeneration on subsequent calls.
This will maintain cache consistency even if the file operations fail.
| console.log("No cache found for", repoOwner, repoName, "generating..."); | ||
| this.generatingTasks[cacheFilePath] = fetchTemplates(repoVersion); | ||
| const categories = await this.generatingTasks[cacheFilePath]; | ||
| this.generatingTasks[cacheFilePath] = null; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling in concurrency control.
If the fetch operation fails, the generatingTasks entry is set to null but the error isn't properly handled. This could lead to inconsistent state where the error propagates but the task tracking is cleared.
console.log("No cache found for", repoOwner, repoName, "generating...");
this.generatingTasks[cacheFilePath] = fetchTemplates(repoVersion);
- const categories = await this.generatingTasks[cacheFilePath];
- this.generatingTasks[cacheFilePath] = null;
+ try {
+ const categories = await this.generatingTasks[cacheFilePath];
+ this.generatingTasks[cacheFilePath] = null;
+ return categories;
+ } catch (error) {
+ this.generatingTasks[cacheFilePath] = null;
+ throw error;
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/api/src/services/external/templates/template-gallery.service.ts around
lines 100 to 103, the current code sets this.generatingTasks[cacheFilePath] to
null after awaiting fetchTemplates, but if fetchTemplates throws an error, the
entry is cleared without proper error handling. To fix this, wrap the await call
in a try-catch block, set this.generatingTasks[cacheFilePath] to null in a
finally block to ensure cleanup, and handle or rethrow the error appropriately
to maintain consistent state and proper error propagation.
715fe81 to
dc7038a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/api/src/services/external/templates/template-processor.service.ts (3)
16-16: Inefficient deep cloning approach may cause performance and reliability issues.The
JSON.parse(JSON.stringify())approach for deep cloning has several drawbacks:
- Performance overhead for large objects
- Loses functions, undefined values, and symbols
- Fails with circular references
- Doesn't preserve prototype chain
Consider using a proper deep cloning utility or restructure to avoid deep cloning:
- mergedCategories.push(JSON.parse(JSON.stringify(category))); + mergedCategories.push({ + ...category, + templates: [...category.templates], + templateSources: [...category.templateSources] + });
22-23: Add input validation for markdown processing methods.The methods assume string input but don't validate the type, which could cause runtime errors if non-string values are passed.
Add type validation:
getTemplateSummary(readme: string): string | null { - if (!readme) return null; + if (!readme || typeof readme !== 'string') return null;Apply similar validation to other methods that process string inputs.
74-76: HTML comment removal regex doesn't handle multi-line comments.The current regex
/<!--.+-->/gwon't match HTML comments that span multiple lines because.doesn't match newline characters by default.Fix the regex to handle multi-line comments:
- return markdown.replace(/<!--.+-->/g, ""); + return markdown.replace(/<!--[\s\S]*?-->/g, "");
🧹 Nitpick comments (2)
apps/api/test/mocks/templates/git/akash-network/awesome-akash/readme.md (1)
1-11: Use consistent heading levelsThe document jumps from an H1 (
#) directly to H3 (###). Markdown-lint flags this (MD001). Align section headings with##to keep a logical outline.-### Official +## Official @@ -### AI - CPU +## AI - CPUapps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/readme.md (1)
3-3: Fix grammatical error.-akash-linuxserver is a mirror of linuxserver docker images that have been converted to akash compliant YAML files. This make it easy for new users to deploy more applications on the [Akash](https://akash.network) network. +akash-linuxserver is a mirror of linuxserver docker images that have been converted to akash compliant YAML files. This makes it easy for new users to deploy more applications on the [Akash](https://akash.network) network.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/template-cache.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (35)
apps/api/scripts/buildAkashTemplatesCache.ts(2 hunks)apps/api/src/app.ts(2 hunks)apps/api/src/routes/v1/index.ts(1 hunks)apps/api/src/routes/v1/templates/byId.ts(0 hunks)apps/api/src/routes/v1/templates/list-full.ts(0 hunks)apps/api/src/routes/v1/templates/list.ts(0 hunks)apps/api/src/services/external/templateReposService.ts(0 hunks)apps/api/src/services/external/templates/template-fetcher.service.ts(1 hunks)apps/api/src/services/external/templates/template-gallery.service.ts(1 hunks)apps/api/src/services/external/templates/template-processor.service.ts(1 hunks)apps/api/src/services/external/templatesCollector.ts(0 hunks)apps/api/src/template/controllers/template/template.controller.ts(1 hunks)apps/api/src/template/http-schemas/template.schema.ts(1 hunks)apps/api/src/template/index.ts(1 hunks)apps/api/src/template/routes/index.ts(1 hunks)apps/api/src/template/routes/templates/templates.router.ts(1 hunks)apps/api/src/template/types/template.ts(1 hunks)apps/api/src/utils/json.ts(1 hunks)apps/api/test/functional/template-cache.spec.ts(1 hunks)apps/api/test/functional/templates.spec.ts(1 hunks)apps/api/test/mocks/templates/cache/akash-network-awesome-akash-cached-sha.json(1 hunks)apps/api/test/mocks/templates/cache/akash-network-cosmos-omnibus-cached-sha.json(1 hunks)apps/api/test/mocks/templates/cache/cryptoandcoffee-akash-linuxserver-cached-sha.json(1 hunks)apps/api/test/mocks/templates/git/akash-network/awesome-akash/alpaca-cpp/index.json(1 hunks)apps/api/test/mocks/templates/git/akash-network/awesome-akash/lunie-lite/index.json(1 hunks)apps/api/test/mocks/templates/git/akash-network/awesome-akash/readme.md(1 hunks)apps/api/test/mocks/templates/git/akash-network/awesome-akash/ssh-ubuntu/index.json(1 hunks)apps/api/test/mocks/templates/git/akash-network/cosmos-omnibus/akash/index.json(1 hunks)apps/api/test/mocks/templates/git/akash-network/cosmos-omnibus/archway/index.json(1 hunks)apps/api/test/mocks/templates/git/akash-network/cosmos-omnibus/index.json(1 hunks)apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/adguardhome-sync/index.json(1 hunks)apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/airsonic-advanced/index.json(1 hunks)apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/airsonic/index.json(1 hunks)apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/readme.md(1 hunks)apps/api/tsconfig.strict.json(1 hunks)
💤 Files with no reviewable changes (5)
- apps/api/src/routes/v1/templates/list.ts
- apps/api/src/routes/v1/templates/list-full.ts
- apps/api/src/services/external/templateReposService.ts
- apps/api/src/routes/v1/templates/byId.ts
- apps/api/src/services/external/templatesCollector.ts
✅ Files skipped from review due to trivial changes (8)
- apps/api/src/routes/v1/index.ts
- apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/airsonic/index.json
- apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/adguardhome-sync/index.json
- apps/api/test/mocks/templates/git/akash-network/cosmos-omnibus/archway/index.json
- apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/airsonic-advanced/index.json
- apps/api/test/mocks/templates/git/akash-network/awesome-akash/ssh-ubuntu/index.json
- apps/api/test/mocks/templates/cache/akash-network-awesome-akash-cached-sha.json
- apps/api/test/mocks/templates/cache/cryptoandcoffee-akash-linuxserver-cached-sha.json
🚧 Files skipped from review as they are similar to previous changes (18)
- apps/api/src/template/index.ts
- apps/api/scripts/buildAkashTemplatesCache.ts
- apps/api/src/app.ts
- apps/api/tsconfig.strict.json
- apps/api/test/mocks/templates/git/akash-network/cosmos-omnibus/akash/index.json
- apps/api/test/mocks/templates/git/akash-network/awesome-akash/lunie-lite/index.json
- apps/api/src/template/routes/index.ts
- apps/api/test/mocks/templates/git/akash-network/awesome-akash/alpaca-cpp/index.json
- apps/api/test/functional/template-cache.spec.ts
- apps/api/src/utils/json.ts
- apps/api/src/template/controllers/template/template.controller.ts
- apps/api/test/mocks/templates/git/akash-network/cosmos-omnibus/index.json
- apps/api/test/functional/templates.spec.ts
- apps/api/src/template/routes/templates/templates.router.ts
- apps/api/src/template/types/template.ts
- apps/api/src/services/external/templates/template-gallery.service.ts
- apps/api/src/template/http-schemas/template.schema.ts
- apps/api/test/mocks/templates/cache/akash-network-cosmos-omnibus-cached-sha.json
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
apps/api/src/services/external/templates/template-processor.service.ts (1)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
apps/api/src/services/external/templates/template-fetcher.service.ts (1)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
🪛 LanguageTool
apps/api/test/mocks/templates/git/akash-network/awesome-akash/readme.md
[grammar] ~5-~5: This phrase is duplicated. You should probably use “Cosmos SDK” only once.
Context: ... --> ### Official - Lunie Wallet for Cosmos SDK - [Cosmos SDK Node](https://github.com/ovrclk/akash-o...
(PHRASE_REPETITION)
apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/readme.md
[grammar] ~3-~3: Possible subject-verb agreement error detected.
Context: ...nverted to akash compliant YAML files. This make it easy for new users to deploy more ap...
(THIS_THAT_AGR)
[duplication] ~12-~12: Possible typo: you repeated a word.
Context: ...AdGuardHome Sync - Airsonic - Airsonic Advanced
(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
apps/api/test/mocks/templates/git/akash-network/awesome-akash/readme.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (3)
apps/api/src/services/external/templates/template-fetcher.service.ts (3)
144-170: Excellent error handling strategy maintains process resilience.The error handling implementation correctly follows the established pattern of catching all errors and returning null rather than breaking the template fetching process. The use of
new Error()objects instead of string literals provides better debugging information while maintaining stability.
335-347: Well-implemented concurrent processing with proper error handling.The
mapConcurrentlymethod provides excellent concurrency control using PromisePool, and the error aggregation strategy ensures that individual failures don't break the entire process while still surfacing critical issues when needed.
267-308: Robust template fetching with external data enrichment.The
fetchOmnibusTemplatesmethod demonstrates good architectural design by:
- Fetching directory contents efficiently
- Enriching template metadata with external chain registry data
- Handling failures gracefully for individual chain data lookups
- Using controlled concurrency for external API calls
This approach ensures the template fetching process remains stable even when external dependencies fail.
stalniy
force-pushed
the
refactor/1273-templates
branch
from
dc7038a to
9aa2c7a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/api/test/mocks/templates/git/akash-network/awesome-akash/readme.md (3)
3-4: Fix heading level to comply with Markdown style guides
### Officialskips one level after the file-level#heading. Use##to keep a proper hierarchy and silencemarkdownlintMD001.-### Official +## Official
5-7: Trim duplicated “Cosmos SDK” wordingThe phrase “Cosmos SDK” appears twice in the bullet — once in the link text and again in the description — triggering the LanguageTool repetition warning and reducing readability.
-- [Lunie Wallet for Cosmos SDK](lunie-lite) +- [Lunie Wallet](lunie-lite) — Cosmos SDK
11-12: Add trailing newline at EOFMany editors and CI linters expect a newline at the end of the file. Add one to avoid future diffs solely for that reason.
-- [Alpaca.cpp](alpaca-cpp) -12 +- [Alpaca.cpp](alpaca-cpp) +apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/readme.md (1)
3-3: Grammar: “This make” → “This makes”.Minor nitpick – fix the verb agreement to keep the README polished.
-This make it easy for new users +This makes it easy for new usersapps/api/test/mocks/templates/cache/cryptoandcoffee-akash-linuxserver-cached-sha.json (1)
34-38: Consider trimming huge README blobs from cached fixtures.Embedding multi-hundred-line READMEs inflates the repo and slows tests.
Possible alternatives:
- Store only a short excerpt plus a pointer to the upstream URL.
- Move bulky strings to a separate compressed fixture loaded on demand.
This keeps the mock cache lightweight while preserving test fidelity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/template-cache.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (35)
apps/api/scripts/buildAkashTemplatesCache.ts(2 hunks)apps/api/src/app.ts(2 hunks)apps/api/src/routes/v1/index.ts(1 hunks)apps/api/src/routes/v1/templates/byId.ts(0 hunks)apps/api/src/routes/v1/templates/list-full.ts(0 hunks)apps/api/src/routes/v1/templates/list.ts(0 hunks)apps/api/src/services/external/templateReposService.ts(0 hunks)apps/api/src/services/external/templates/template-fetcher.service.ts(1 hunks)apps/api/src/services/external/templates/template-gallery.service.ts(1 hunks)apps/api/src/services/external/templates/template-processor.service.ts(1 hunks)apps/api/src/services/external/templatesCollector.ts(0 hunks)apps/api/src/template/controllers/template/template.controller.ts(1 hunks)apps/api/src/template/http-schemas/template.schema.ts(1 hunks)apps/api/src/template/index.ts(1 hunks)apps/api/src/template/routes/index.ts(1 hunks)apps/api/src/template/routes/templates/templates.router.ts(1 hunks)apps/api/src/template/types/template.ts(1 hunks)apps/api/src/utils/json.ts(1 hunks)apps/api/test/functional/template-cache.spec.ts(1 hunks)apps/api/test/functional/templates.spec.ts(1 hunks)apps/api/test/mocks/templates/cache/akash-network-awesome-akash-cached-sha.json(1 hunks)apps/api/test/mocks/templates/cache/akash-network-cosmos-omnibus-cached-sha.json(1 hunks)apps/api/test/mocks/templates/cache/cryptoandcoffee-akash-linuxserver-cached-sha.json(1 hunks)apps/api/test/mocks/templates/git/akash-network/awesome-akash/alpaca-cpp/index.json(1 hunks)apps/api/test/mocks/templates/git/akash-network/awesome-akash/lunie-lite/index.json(1 hunks)apps/api/test/mocks/templates/git/akash-network/awesome-akash/readme.md(1 hunks)apps/api/test/mocks/templates/git/akash-network/awesome-akash/ssh-ubuntu/index.json(1 hunks)apps/api/test/mocks/templates/git/akash-network/cosmos-omnibus/akash/index.json(1 hunks)apps/api/test/mocks/templates/git/akash-network/cosmos-omnibus/archway/index.json(1 hunks)apps/api/test/mocks/templates/git/akash-network/cosmos-omnibus/index.json(1 hunks)apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/adguardhome-sync/index.json(1 hunks)apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/airsonic-advanced/index.json(1 hunks)apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/airsonic/index.json(1 hunks)apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/readme.md(1 hunks)apps/api/tsconfig.strict.json(1 hunks)
💤 Files with no reviewable changes (5)
- apps/api/src/routes/v1/templates/list.ts
- apps/api/src/routes/v1/templates/list-full.ts
- apps/api/src/routes/v1/templates/byId.ts
- apps/api/src/services/external/templateReposService.ts
- apps/api/src/services/external/templatesCollector.ts
✅ Files skipped from review due to trivial changes (5)
- apps/api/src/routes/v1/index.ts
- apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/airsonic/index.json
- apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/airsonic-advanced/index.json
- apps/api/test/mocks/templates/git/akash-network/awesome-akash/ssh-ubuntu/index.json
- apps/api/test/mocks/templates/git/akash-network/cosmos-omnibus/index.json
🚧 Files skipped from review as they are similar to previous changes (22)
- apps/api/src/app.ts
- apps/api/scripts/buildAkashTemplatesCache.ts
- apps/api/src/template/index.ts
- apps/api/src/template/routes/index.ts
- apps/api/tsconfig.strict.json
- apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/adguardhome-sync/index.json
- apps/api/test/mocks/templates/git/akash-network/cosmos-omnibus/akash/index.json
- apps/api/test/mocks/templates/git/akash-network/awesome-akash/alpaca-cpp/index.json
- apps/api/test/mocks/templates/git/akash-network/cosmos-omnibus/archway/index.json
- apps/api/src/utils/json.ts
- apps/api/test/functional/template-cache.spec.ts
- apps/api/test/mocks/templates/git/akash-network/awesome-akash/lunie-lite/index.json
- apps/api/src/template/controllers/template/template.controller.ts
- apps/api/test/mocks/templates/cache/akash-network-cosmos-omnibus-cached-sha.json
- apps/api/test/functional/templates.spec.ts
- apps/api/src/template/types/template.ts
- apps/api/src/template/routes/templates/templates.router.ts
- apps/api/src/services/external/templates/template-gallery.service.ts
- apps/api/test/mocks/templates/cache/akash-network-awesome-akash-cached-sha.json
- apps/api/src/services/external/templates/template-processor.service.ts
- apps/api/src/services/external/templates/template-fetcher.service.ts
- apps/api/src/template/http-schemas/template.schema.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
🪛 Gitleaks (8.26.0)
apps/api/test/mocks/templates/cache/cryptoandcoffee-akash-linuxserver-cached-sha.json
39-39: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
apps/api/test/mocks/templates/git/akash-network/awesome-akash/readme.md
[grammar] ~5-~5: This phrase is duplicated. You should probably use “Cosmos SDK” only once.
Context: ... --> ### Official - Lunie Wallet for Cosmos SDK - [Cosmos SDK Node](https://github.com/ovrclk/akash-o...
(PHRASE_REPETITION)
apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/readme.md
[grammar] ~3-~3: Possible subject-verb agreement error detected.
Context: ...nverted to akash compliant YAML files. This make it easy for new users to deploy more ap...
(THIS_THAT_AGR)
[duplication] ~12-~12: Possible typo: you repeated a word.
Context: ...AdGuardHome Sync - Airsonic - Airsonic Advanced
(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
apps/api/test/mocks/templates/git/akash-network/awesome-akash/readme.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ 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: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (1)
apps/api/test/mocks/templates/git/cryptoandcoffee/akash-linuxserver/readme.md (1)
1-14: Confirm upstream-license compatibility before vendoring full README.The file embeds verbatim upstream content from LinuxServer. Double-check that:
- The upstream project’s license permits redistribution in this repo.
- Required attribution notices are preserved.
If redistribution is not clearly allowed, replace with a stub + link instead.
| "deploy": "version: \"2.0\"\n\nservices:\n airsonic-advanced:\n image: linuxserver/airsonic-advanced\n expose:\n - port: 4040\n as: 4040\n to:\n - global: true\n - port: 80\n as: 80\n to:\n - global: true\n\n env:\n - CONTEXT_PATH=\n - PASSWORD=ff06ff28e92b975c5b6745d3ff088422\n - JAVA_OPTS=\n - JAVA_OPTS=\"-Xmx256m\n - PGID=1000\n - PUID=1000\n - TZ=Etc/UTC\n\nprofiles:\n compute:\n airsonic-advanced:\n resources:\n cpu:\n units: 4.0\n memory:\n size: 2.5Gi\n storage:\n size: 32Gi\n placement:\n akash:\n signedBy:\n anyOf:\n - \"akash1365yvmc4s7awdyj3n2sav7xfx76adc6dnmlx63\"\n - \"akash18qa2a2ltfyvkyj0ggj3hkvuj6twzyumuaru9s4\"\n pricing:\n airsonic-advanced:\n denom: uakt\n amount: 100000\ndeployment:\n airsonic-advanced:\n akash:\n profile: airsonic-advanced\n count: 1\n", | ||
| "persistentStorageEnabled": false, |
There was a problem hiding this comment.
Second hard-coded credential – sanitize likewise.
Same issue for the Airsonic Advanced template:
- - PASSWORD=ff06ff28e92b975c5b6745d3ff088422
+ - PASSWORD=<redacted-password>📝 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.
| "deploy": "version: \"2.0\"\n\nservices:\n airsonic-advanced:\n image: linuxserver/airsonic-advanced\n expose:\n - port: 4040\n as: 4040\n to:\n - global: true\n - port: 80\n as: 80\n to:\n - global: true\n\n env:\n - CONTEXT_PATH=\n - PASSWORD=ff06ff28e92b975c5b6745d3ff088422\n - JAVA_OPTS=\n - JAVA_OPTS=\"-Xmx256m\n - PGID=1000\n - PUID=1000\n - TZ=Etc/UTC\n\nprofiles:\n compute:\n airsonic-advanced:\n resources:\n cpu:\n units: 4.0\n memory:\n size: 2.5Gi\n storage:\n size: 32Gi\n placement:\n akash:\n signedBy:\n anyOf:\n - \"akash1365yvmc4s7awdyj3n2sav7xfx76adc6dnmlx63\"\n - \"akash18qa2a2ltfyvkyj0ggj3hkvuj6twzyumuaru9s4\"\n pricing:\n airsonic-advanced:\n denom: uakt\n amount: 100000\ndeployment:\n airsonic-advanced:\n akash:\n profile: airsonic-advanced\n count: 1\n", | |
| "persistentStorageEnabled": false, | |
| - CONTEXT_PATH= | |
| - - PASSWORD=ff06ff28e92b975c5b6745d3ff088422 | |
| + - PASSWORD=<redacted-password> | |
| - JAVA_OPTS= |
🧰 Tools
🪛 Gitleaks (8.26.0)
54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In
apps/api/test/mocks/templates/cache/cryptoandcoffee-akash-linuxserver-cached-sha.json
around lines 54 to 55, there is a hard-coded password credential in the "deploy"
field for the Airsonic Advanced service. Replace this hard-coded password with a
sanitized placeholder or environment variable reference to avoid exposing
sensitive information in the template.
| "deploy": "version: \"2.0\"\n\nservices:\n adguardhome-sync:\n image: linuxserver/adguardhome-sync\n expose:\n - port: 8080\n as: 8080\n to:\n - global: true\n - port: 80\n as: 80\n to:\n - global: true\n\n env:\n - CONFIGFILE=/config/adguardhome-sync.yaml\n - PASSWORD=01c166aa501a8ea9f61a3d86028fcafb\n - PGID=1000\n - PUID=1000\n - TZ=Etc/UTC\n\nprofiles:\n compute:\n adguardhome-sync:\n resources:\n cpu:\n units: 4.0\n memory:\n size: 2.5Gi\n storage:\n size: 32Gi\n placement:\n akash:\n signedBy:\n anyOf:\n - \"akash1365yvmc4s7awdyj3n2sav7xfx76adc6dnmlx63\"\n - \"akash18qa2a2ltfyvkyj0ggj3hkvuj6twzyumuaru9s4\"\n pricing:\n adguardhome-sync:\n denom: uakt\n amount: 100000\ndeployment:\n adguardhome-sync:\n akash:\n profile: adguardhome-sync\n count: 1\n", | ||
| "persistentStorageEnabled": false, |
There was a problem hiding this comment.
Hard-coded password detected – replace with a placeholder to avoid false-positive secret scans.
The PASSWORD=01c166aa501a8ea9f61a3d86028fcafb value trips Gitleaks.
Even if it’s benign fixture data, swap it for a clearly fake placeholder:
- - PASSWORD=01c166aa501a8ea9f61a3d86028fcafb
+ - PASSWORD=<redacted-password>📝 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.
| "deploy": "version: \"2.0\"\n\nservices:\n adguardhome-sync:\n image: linuxserver/adguardhome-sync\n expose:\n - port: 8080\n as: 8080\n to:\n - global: true\n - port: 80\n as: 80\n to:\n - global: true\n\n env:\n - CONFIGFILE=/config/adguardhome-sync.yaml\n - PASSWORD=01c166aa501a8ea9f61a3d86028fcafb\n - PGID=1000\n - PUID=1000\n - TZ=Etc/UTC\n\nprofiles:\n compute:\n adguardhome-sync:\n resources:\n cpu:\n units: 4.0\n memory:\n size: 2.5Gi\n storage:\n size: 32Gi\n placement:\n akash:\n signedBy:\n anyOf:\n - \"akash1365yvmc4s7awdyj3n2sav7xfx76adc6dnmlx63\"\n - \"akash18qa2a2ltfyvkyj0ggj3hkvuj6twzyumuaru9s4\"\n pricing:\n adguardhome-sync:\n denom: uakt\n amount: 100000\ndeployment:\n adguardhome-sync:\n akash:\n profile: adguardhome-sync\n count: 1\n", | |
| "persistentStorageEnabled": false, | |
| "deploy": "version: \"2.0\"\n\nservices:\n adguardhome-sync:\n image: linuxserver/adguardhome-sync\n expose:\n - port: 8080\n as: 8080\n to:\n - global: true\n - port: 80\n as: 80\n to:\n - global: true\n\n env:\n - CONFIGFILE=/config/adguardhome-sync.yaml\n - PASSWORD=<redacted-password>\n - PGID=1000\n - PUID=1000\n - TZ=Etc/UTC\n\nprofiles:\n compute:\n adguardhome-sync:\n resources:\n cpu:\n units: 4.0\n memory:\n size: 2.5Gi\n storage:\n size: 32Gi\n placement:\n akash:\n signedBy:\n anyOf:\n - \"akash1365yvmc4s7awdyj3n2sav7xfx76adc6dnmlx63\"\n - \"akash18qa2a2ltfyvkyj0ggj3hkvuj6twzyumuaru9s4\"\n pricing:\n adguardhome-sync:\n denom: uakt\n amount: 100000\ndeployment:\n adguardhome-sync:\n akash:\n profile: adguardhome-sync\n count: 1\n", | |
| "persistentStorageEnabled": false, |
🧰 Tools
🪛 Gitleaks (8.26.0)
39-39: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In
apps/api/test/mocks/templates/cache/cryptoandcoffee-akash-linuxserver-cached-sha.json
around lines 39 to 40, the deploy section contains a hard-coded password value
"PASSWORD=01c166aa501a8ea9f61a3d86028fcafb" which triggers secret scanning
tools. Replace this actual password with a clearly fake placeholder string like
"PASSWORD=FAKE_PASSWORD" to prevent false-positive secret scans while keeping
the fixture data valid.
| const expectCategory = (result: GetTemplatesFullResponse, expectedCategory: string, expectedTemplateIds: string[]) => { | ||
| const category = result.find((c: { title: string }) => c.title === expectedCategory); | ||
| expect(category.templates).toHaveLength(expectedTemplateIds.length); | ||
| const templateIds = category.templates.map(({ id }) => id); | ||
| templateIds.forEach(templateId => { | ||
| expect(expectedTemplateIds).toContain(templateId); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
suggestion(non-blocking): introducing reusable assertion function is bad idea because when test fails, test runner will display you part of the stack trace from reusable function which won't be informative/helpful.
Better to rewrite this into shorter form or create a custom expect matcher. For example:
expect(findCategory(result, expectedCategory)?.templates).toHaveLength(5);
expect(findCategory(result, expectedCategory)?.templates.map(t => t.id)).toEqual(expect.arrayContaining(expectedTemplateIds));
2 participants
closes #1273
Summary by CodeRabbit
New Features
/v1to retrieve deployment templates, including full lists, filtered lists, and individual templates by ID.Refactor
Bug Fixes
Tests
Chores