fix: replaces fetch API with octokit.getContent#2570
Merged
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
📝 WalkthroughWalkthroughReplaces direct global fetch with Octokit REST raw content retrieval in template fetching; adds ignore-by-keywords and ignore-by-paths options propagated through processing; updates tests and consumers to use Octokit only. Also bumps Changes
Sequence Diagram(s)sequenceDiagram
participant Gallery as TemplateGalleryService
participant Fetcher as TemplateFetcherService
participant Octo as Octokit (REST)
participant Proc as TemplateProcessorService
participant Log as LoggerService
Gallery->>Fetcher: instantiate(templateProcessor, logger, octokit, options)
Gallery->>Fetcher: fetchTemplatesInfo(categories, options)
Fetcher->>Log: debug "processing category"
Fetcher->>Octo: repos.getContent(owner, repo, path, mediaType=raw, ref?)
Octo-->>Fetcher: raw file content / directory listing
Fetcher->>Proc: process template files (README, deploy, config)
Proc-->>Fetcher: processed template metadata
Fetcher->>Log: warn/info on ignored templates or errors
Fetcher-->>Gallery: categories with template info
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (78.01%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2570 +/- ##
==========================================
- Coverage 50.86% 49.77% -1.09%
==========================================
Files 1071 1025 -46
Lines 29901 29088 -813
Branches 6630 6573 -57
==========================================
- Hits 15209 14480 -729
+ Misses 14388 14252 -136
- Partials 304 356 +52
🚀 New features to boost your workflow:
|
baktun14
previously approved these changes
Jan 26, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/template/services/template-fetcher/template-fetcher.service.ts (1)
83-101: Add type guard for response.data in fetchFileContent
mediaType.format="raw"is the correct way to fetch raw content in Octokit v22, butString(response.data)can mask errors if the response unexpectedly returns a non-string value. Add a type check before returning, matching the pattern used infetchDirectoryContent:Suggested change
- if (response.status !== 200) { - throw new Error(`Failed to fetch content from ${owner}/${repo}/${path}`); - } - - return String(response.data); + if (response.status !== 200 || typeof response.data !== "string") { + throw new Error(`Failed to fetch content from ${owner}/${repo}/${path}`); + } + + return response.data;
🤖 Fix all issues with AI agents
In `@apps/api/package.json`:
- Line 66: The dependency bump to "@octokit/rest": "^22.0.1" requires Node.js
≥20 and ESM-first imports; verify the runtime and code accordingly by ensuring
the project's Node target is >=20 (update runtime config/CI), convert any
CommonJS requires of "@octokit/rest" to ESM-style imports (e.g., replace
require(...) with import { Octokit } from "@octokit/rest" where used), and if
this is a TypeScript project update tsconfig.json (adjust "module" and
"moduleResolution" to "node16" or "nodenext" as appropriate) so TypeScript
resolves the package's conditional exports; check all usages of Octokit and
build/CI settings to prevent runtime import/resolution failures.
stalniy
force-pushed
the
fix/fetching-templates
branch
from
5abf336 to
3b3876f
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@apps/api/src/template/services/template-fetcher/template-fetcher.service.spec.ts`:
- Around line 631-639: The test helper function setup currently has no parameter
and should accept a single inline-typed options parameter; update the setup
signature to take one object parameter (with an inline type describing optional
templateProcessor: TemplateProcessorService, logger: LoggerService, and octokit:
Octokit) with defaults that create mocks, then use those values to construct the
TemplateFetcherService instance and return them (referencing setup,
TemplateFetcherService, templateProcessor, logger, octokit to locate the code).
In `@apps/api/src/template/services/template-fetcher/template-fetcher.service.ts`:
- Around line 168-179: The comparison against options.ignoreByPaths uses exact
equality and can miss variants like "./jenkins" or "jenkins/"; update the check
in the template-fetcher logic (the block that logs
TEMPLATE_SOURCE_PROCESSING_SKIPPED and any similar check later) to normalize
both sides before comparing: create/inline a small normalizer (e.g., trim
whitespace, remove leading "./", remove trailing slashes, and optionally
lowercase) and compare normalize(templateSource.path) === normalize(ignorePath)
or use .some() over normalized ignore entries; apply the same normalization to
the duplicate check around lines 345-356 so both locations use the same
normalizePath helper and behavior.
🧹 Nitpick comments (1)
apps/api/src/template/services/template-fetcher/template-fetcher.service.ts (1)
125-149: Avoid double-fetching README when keyword filtering is enabled.README content is fetched once for keyword filtering and then fetched again inside
processTemplateSource. This doubles GitHub API calls per template and eats rate limits. Consider passing the already-fetched README intoprocessTemplateSourceto reuse it.♻️ Suggested refactor to reuse README
- private async processTemplateSource( - templateSource: TemplateSource, - directoryItems: GithubDirectoryItem[], - options: { includeConfigJson?: boolean } - ): Promise<any> { + private async processTemplateSource( + templateSource: TemplateSource, + directoryItems: GithubDirectoryItem[], + options: { includeConfigJson?: boolean }, + prefetchedReadme?: string | null + ): Promise<any> { try { if (templateSource.path.startsWith("http:") || templateSource.path.startsWith("https:")) { throw new Error("Absolute URL not supported"); } - const readme = await this.findFileContentAsync("README.md", directoryItems, templateSource); + const readme = prefetchedReadme ?? (await this.findFileContentAsync("README.md", directoryItems, templateSource)); const [deploy, guide, configJsonText] = await Promise.all([ this.findFileContentAsync(["deploy.yaml", "deploy.yml"], directoryItems, templateSource), this.findFileContentAsync("GUIDE.md", directoryItems, templateSource), options.includeConfigJson ? this.findFileContentAsync("config.json", directoryItems, templateSource) : Promise.resolve(null) ]);- return this.processTemplateSource(templateSource, directoryItems, options); + return this.processTemplateSource(templateSource, directoryItems, options, readme);Also applies to: 190-203
apps/api/src/template/services/template-fetcher/template-fetcher.service.spec.ts
Show resolved
Hide resolved
apps/api/src/template/services/template-fetcher/template-fetcher.service.ts
Show resolved
Hide resolved
stalniy
force-pushed
the
fix/fetching-templates
branch
7 times, most recently
from
78f6d43 to
bd4c1e6
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.codecov.yml:
- Line 3: The ignore glob currently only excludes nested route directories via
"apps/api/src/*/routes/**/*.ts" but misses the top-level routers directory, so
update the coverage ignore list to also include the pattern
"apps/api/src/routers/**/*.ts" (alongside the existing
"apps/api/src/*/routes/**/*.ts") so the seven top-level TypeScript router files
remain excluded from coverage.
In `@packages/docker/Dockerfile.node`:
- Around line 20-21: The current Dockerfile.node step runs `npm version ...` and
then mutates `package-lock.json` only by setting `lockfile.version` to "1.0.0";
update the inline Node script invoked by `node -e` to also set the
workspace-specific lockfile entry (i.e.,
`lockfile.packages['$WORKSPACE'].version`) to "1.0.0" so the workspace package
version in npm lockfile v3 is reset as in Dockerfile.nextjs; locate the `npm
version --allow-same-version 1.0.0 -w $WORKSPACE --no-workspaces-update` line
and the following `node -e "const fs=...; const lockfile=...;
fs.writeFileSync(...)"` invocation and modify the script to load
package-lock.json, set both `lockfile.version` and
`lockfile.packages['$WORKSPACE'].version` to "1.0.0", then write the file back.
bd4c1e6 to
d421ba5
Compare
This was referenced Jan 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
in order to use the GH_TOKEN to fetch files content because direct
fetchAPI call timeouts sometimesSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.