Conversation
|
Warning Rate limit exceeded@Viren070 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 54 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis update restructures the codebase, consolidating multiple packages into a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Core
participant PresetManager
participant AddonPreset
participant Wrapper
participant Proxy
participant Formatter
participant StremioTransformer
User->>Core: Submit stream/catalog/meta/subtitle request
Core->>PresetManager: Retrieve applicable presets
PresetManager->>AddonPreset: Generate addon(s) with options
Core->>Wrapper: Fetch manifest/resources/streams from addon(s)
Wrapper->>AddonPreset: Use parser to normalise streams
Core->>Core: Filter, deduplicate, sort, limit streams
Core->>Proxy: Proxy streams if configured/applicable
Core->>Formatter: Format streams with selected/custom formatter
Core->>StremioTransformer: Transform response to Stremio format
StremioTransformer->>User: Return formatted response
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Poem
✨ 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 (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (10)
packages/core/src/db/schemas.ts (3)
4-18:z.enum(constants.X)still expects a readonly tuple – compile will failEarlier feedback noted that passing a plain
string[]toz.enumviolates the requiredReadonly<[...]>signature unless the constant is declaredas const. Please apply the fix consistently:-const ServiceIds = z.enum(constants.SERVICES); +const ServiceIds = z.enum([...constants.SERVICES] as const);Repeat for every
z.enum(constants.*)in this file.
43-50: Plain-text credential storage
StreamProxyConfig.credentialsremains an unencrypted string. Storing secrets unhashed in the DB/logs is a security liability; hash or encrypt before persistence and redact on serialisation.
70-84:SizeFilterallows zero-byte files – reconsider validationKeeping
min(0)undermines the purpose of a size filter. Usemin(1)ornonnegative()as previously suggested to enforce meaningful limits.packages/core/src/main.ts (5)
96-101: CallcheckInitialised()before doing any work
getStreams()can be called directly by consumers.
Without an initial safety-net the whole pipeline may crash ifinitialise()was forgotten. We already raised this in an earlier review – please add the guard as the first statement and return early if it throws.public async getStreams( id: string, type: string, preCaching: boolean = false ): Promise<AIOStreamsResponse<ParsedStream[]>> { + this.checkInitialised(); logger.info(`Handling stream request`, { type, id });
1104-1131:skipReasonsis still missing the AudioChannel keys → runtime crashLater in the filter pipeline (
~1652,~1674) you increment
skipReasons.excludedAudioChannel/requiredAudioChannel, but they are not initialised here.
Accessing.totalor.detailsonundefinedwill throw and break filtering for every request.excludedAudioTag: { total: 0, details: {} }, requiredAudioTag: { total: 0, details: {} }, + excludedAudioChannel: { total: 0, details: {} }, + requiredAudioChannel: { total: 0, details: {} },
1376-1384: Unsafe optional-chaining on.length– silent crashes remainPatterns like
file?.visualTags.length/file?.audioTags.lengthdereference.lengtheven when the array isundefined, producingTypeError: Cannot read properties of undefined (reading 'length').Refactor to double–optional chain (or coalesce) everywhere:
- (file?.visualTags.length ? file.visualTags : ['Unknown']).includes(tag) + ((file?.visualTags?.length ?? 0) > 0 + ? file!.visualTags! + : ['Unknown'] + ).includes(tag)A quick grep for
\?\.\w+\.lengthstill shows 10+ occurrences.
Please fix them or introduce a helper such assafeArray<T>(a?: T[]): T[].Also applies to: 1570-1580
2040-2056: Wrong truthiness check treats-1as “found” – sorting bug
aProviderIndex && bProviderIndexis truthy even if both are-1, so the “provider priority” branch executes when neither provider is configured, yielding unstable order.- if (aProviderIndex && bProviderIndex && aProviderIndex !== bProviderIndex) { + if ( + aProviderIndex !== -1 && + bProviderIndex !== -1 && + aProviderIndex !== bProviderIndex + ) { return aProviderIndex - bProviderIndex; }
2273-2280: PossibleTypeErrorwhensortCriteriais undefined
this.userData.sortCriteriais optional, yet you dereference.global,.cached,.uncachedwithout checks. The same issue was noted previously.- let sortCriteria = this.userData.sortCriteria.global; + const sc = this.userData.sortCriteria ?? {}; + let sortCriteria = sc.global ?? [];Apply the same defensive pattern for
cachedSortCriteria/uncachedSortCriteriaand the movie/series overrides below.packages/core/src/utils/constants.ts (2)
87-96: Constructor parameter order issue still present – risk of accidental mis-orderingPrevious review already highlighted that
statusCodecomes beforemessage. Nothing changed, so callers supplying only a custom message will still accidentally pass it as a status code:new APIError(ErrorCode.USER_ERROR, 'Custom message'); // ❌The safer order is
code, message?, statusCode?or an options object.
475-487:⚠️ Potential issue
passwordoption type missing – compilation will break wherever credentials are validated
SERVICE_DETAILSrepeatedly usestype: 'password', yetPASSWORD_OPTION_TYPEis not declared and the literal is absent fromOPTION_TYPES.
OptionDefinition(see db/schemas.ts) most likely doesz.enum(OPTION_TYPES), so every credential definition using'password'will now violate the schema.const STRING_OPTION_TYPE = 'string'; const NUMBER_OPTION_TYPE = 'number'; const BOOLEAN_OPTION_TYPE = 'boolean'; const SELECT_OPTION_TYPE = 'select'; const MULTI_SELECT_OPTION_TYPE = 'multi-select'; +const PASSWORD_OPTION_TYPE = 'password'; // ← add const OPTION_TYPES = [ STRING_OPTION_TYPE, NUMBER_OPTION_TYPE, BOOLEAN_OPTION_TYPE, SELECT_OPTION_TYPE, MULTI_SELECT_OPTION_TYPE, + PASSWORD_OPTION_TYPE, // ← add ] as const;Don’t forget the export at the bottom.
🧹 Nitpick comments (1)
packages/core/src/utils/constants.ts (1)
98-105: Minor completeness nit – standardForwardedheader omittedConsider appending plain
Forwarded(RFC 7239) so consumers that forward this list do not miss it:'Forwarded-For', + 'Forwarded',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.env.sample(1 hunks)packages/core/src/db/schemas.ts(1 hunks)packages/core/src/formatters/base.ts(1 hunks)packages/core/src/formatters/predefined.ts(1 hunks)packages/core/src/main.ts(1 hunks)packages/core/src/utils/constants.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/formatters/predefined.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/core/src/utils/constants.ts (1)
packages/core/src/db/schemas.ts (1)
Option(199-199)
🪛 Biome (1.9.4)
packages/core/src/main.ts
[error] 214-214: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 281-281: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 640-641: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1061-1062: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 2413-2415: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2508-2511: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (12)
packages/core/src/db/schemas.ts (5)
4-18:z.enum(constants.X)is still fed a mutable array – compile will explodeAll
z.enum(constants.*)calls rely onconstants.Xbeing a readonly tuple. Unless every exported constant is declaredas const, the build will fail with TS 2345. This was raised in the previous review and the code is unchanged.
42-50: Plain-text proxy credentials – same security concern
StreamProxyConfig.credentialsis still stored/read as a raw string. A database leak exposes secrets instantly. Please hash, encrypt, or otherwise protect this field and mark it sensitive in serialisation.
70-84: Size filter allows zero-byte files – validation still too lax
z.number().min(0)permits a lower bound of zero, which makes little sense for torrent/file sizes. Prior review suggestedmin(1)or.nonnegative(). Consider tightening the constraint.
91-95: Service credentials stored as plain strings
ServiceSchema.credentialskeeps clear-text values, repeating the earlier security red-flag. Encrypt or hash before persisting and treat as sensitive during logging.
357-367:TABLESis still a stub – users table onlyOnly the
USERStable schema is defined, which suggests the abstraction isn’t finished (sessions, cache, etc., are missing). Either flesh out the constant or move individual DDL to focused files.packages/core/src/main.ts (3)
1104-1131:⚠️ Potential issueAdd missing
audioChannelcounters to preventundefinedaccess
skipReasonsis later updated forexcludedAudioChannel/requiredAudioChannel(e.g. lines 1715 & 1737).
Because the keys are not initialised here, those increments dereferenceundefined, crashing the filter pipeline.excludedAudioTag: { total: 0, details: {} }, requiredAudioTag: { total: 0, details: {} }, + excludedAudioChannel: { total: 0, details: {} }, + requiredAudioChannel: { total: 0, details: {} },
2321-2330:⚠️ Potential issue
sortCriteriaaccess may beundefined– guard before dereference
userData.sortCriteriais optional, yet the code immediately dereferences.global,.cached, and.uncached, leading to a crash for users who omit sort settings.- let sortCriteria = this.userData.sortCriteria.global; - let cachedSortCriteria = this.userData.sortCriteria.cached; - let uncachedSortCriteria = this.userData.sortCriteria.uncached; + const sc = this.userData.sortCriteria ?? {}; + let sortCriteria = sc.global ?? []; + let cachedSortCriteria = sc.cached ?? []; + let uncachedSortCriteria= sc.uncached ?? [];Apply the same defensive access further down when overriding for
movie/series.
96-104: 🛠️ Refactor suggestionCall
checkInitialised()at the start ofgetStreams
getStreamsis publicly exposed and assumesinitialise()has already been invoked.
Without a guard, accidental misuse leads to subtle runtime errors later in the pipeline.public async getStreams( id: string, type: string, preCaching: boolean = false ): Promise<AIOStreamsResponse<ParsedStream[]>> { + this.checkInitialised(); logger.info(`Handling stream request`, { type, id });packages/core/src/utils/constants.ts (4)
87-93: 🛠️ Refactor suggestionAPIError constructor’s parameter order is still counter-intuitive
statusCodeprecedesmessage, so passing only a custom message still forces callers to supply a (redundant) status code, inviting mistakes.new APIError(ErrorCode.USER_ERROR, 'Something went wrong'); // 2nd arg = statusCodeRefactor the signature (or accept an options object) so
messagecomes beforestatusCode, or remove the latter altogether since it can be derived fromcode.
489-493:⚠️ Potential issue
DEDUPLICATOR_KEYSis still not exportedYou declared the constant but forgot to append it to the export block, so downstream modules cannot reuse it.
Add it to the export list (see comment on lines 873-911).
475-487:⚠️ Potential issueMissing
passwordoption type ⇒ compilation will breakCredentials below use
type: 'password', yetPASSWORD_OPTION_TYPEis not declared and'password'is missing fromOPTION_TYPES.
Add the constant, include it inOPTION_TYPES, and export it.const STRING_OPTION_TYPE = 'string'; const NUMBER_OPTION_TYPE = 'number'; const BOOLEAN_OPTION_TYPE = 'boolean'; const SELECT_OPTION_TYPE = 'select'; const MULTI_SELECT_OPTION_TYPE = 'multi-select'; +const PASSWORD_OPTION_TYPE = 'password'; const OPTION_TYPES = [ STRING_OPTION_TYPE, NUMBER_OPTION_TYPE, BOOLEAN_OPTION_TYPE, SELECT_OPTION_TYPE, MULTI_SELECT_OPTION_TYPE, + PASSWORD_OPTION_TYPE, ] as const;
873-911:⚠️ Potential issueExport list is incomplete – leaks types & breaks re-use
Please extend the export block to include the newly required symbols:
MULTI_SELECT_OPTION_TYPE, + PASSWORD_OPTION_TYPE, HEADERS_FOR_IP_FORWARDING, + PROXY_SERVICE_DETAILS, + DEDUPLICATOR_KEYS, };This resolves the missing-export issues noted above.
🧹 Nitpick comments (2)
packages/core/src/db/schemas.ts (2)
22-28: Schema value and type share the same identifier – rename for clarityA value (
const SortCriterion) and a type (type SortCriterion) coexist under the same name. Although legal in TS, it is easy to confuse the runtime schema with the compile-time type. Renaming the value toSortCriterionSchemakeeps the mental model clear.-const SortCriterion = z.object({ +const SortCriterionSchema = z.object({ key: z.enum(constants.SORT_CRITERIA), direction: z.enum(constants.SORT_DIRECTIONS), }); -export type SortCriterion = z.infer<typeof SortCriterion>; +export type SortCriterion = z.infer<typeof SortCriterionSchema>;Follow-up: update the usages (e.g.
sortCriteria.global) accordingly.
1-777: File has grown into a 770-line monolith – consider splittingThis single module mixes:
• primitive enums
• option/stream schemas
• user config, manifest, DB DDL, API responsesNavigation and git-blame become painful. A lightweight refactor into thematic files (e.g.,
user.ts,stream.ts,manifest.ts,responses.ts) will improve maintainability without functional change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/core/src/db/schemas.ts(1 hunks)packages/core/src/main.ts(1 hunks)packages/core/src/utils/constants.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/core/src/utils/constants.ts (1)
packages/core/src/db/schemas.ts (1)
Option(199-199)
🪛 Biome (1.9.4)
packages/core/src/main.ts
[error] 214-214: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 281-281: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 640-641: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1061-1062: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 2461-2463: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2556-2559: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (11)
packages/core/src/utils/http.ts (2)
16-29:makeUrlLogSafemasking logic still suffers from original flaws
The condition!component.includes('.')still prevents masking of long path segments that legitimately contain a dot (e.g. versioned API paths).
The negative-look-behind regex forpassword=is also fragile and won’t work in older Node versions.
76-112: Proxy decision algorithm problems remain unfixed
The earlier review pointed out: rule-order overriding, wildcard mishandling, missingbreak. None of these have been addressed – the logic can still flip-flop on later rules and give surprising results.packages/core/src/transformers/stremio.ts (1)
304-307:createDynamicErrorstill returnsany, losing type safety
Previous feedback recommended overloads or a discriminated union. Leaving it asanyundermines the benefits of the exhaustive switch you’ve written.packages/core/src/db/schemas.ts (4)
4-17:z.enum(constants.X)will not compile unless the constant is a readonly tuple
Ifconstants.SERVICES(and others) is a plainstring[], TypeScript will error. Cast toas constor spread into a tuple.
70-84: Size filters still allow zero-byte files
z.number().min(0)contradicts the earlier suggestion – usemin(1)ornonnegative()based on the real requirement.
42-50: Credentials stored as plain text
credentialsfields remain un-hashed strings. A DB leak exposes user secrets. Encrypt or hash before persistence and mark as sensitive to avoid accidental logging.Also applies to: 91-95
358-368:TABLESconstant still lists onlyUSERS
The helper loses usefulness if other tables (sessions, cache, etc.) aren’t declared. Either flesh it out or move schema strings to a dedicated file.packages/core/src/main.ts (4)
101-108:⚠️ Potential issueCall
checkInitialised()at the start ofgetStreams
getStreamsis public and can be invoked beforeinitialise()has run, leading to subtle runtime failures later in the pipeline.
Add the guard immediately after logging:logger.info(`Handling stream request`, { type, id }); + this.checkInitialised();
2545-2550:⚠️ Potential issueVariable declared in
switchcase leaks scope
const index = …is declared directly inside thecase 'service'clause without braces, violatingnoSwitchDeclarationsand risking name collisions with subsequent cases.- case 'service': - const index = userData.services?.findIndex( + case 'service': { + const index = userData.services?.findIndex( (service) => service.id === stream.service?.id ); return multiplier * -(index === -1 ? 0 : (index ?? 0)); + }
2336-2340: 🛠️ Refactor suggestionDirect dereference of optional
sortCriteriawill crash when absent
this.userData.sortCriteriais optional, yet the code immediately reads.global,.cached,.uncached. If a user omits the field the call path throws.Guard with defaults:
- let sortCriteria = this.userData.sortCriteria.global; - let cachedSortCriteria = this.userData.sortCriteria.cached; - let uncachedSortCriteria = this.userData.sortCriteria.uncached; + const sc = this.userData.sortCriteria ?? {}; + let sortCriteria = sc.global ?? []; + let cachedSortCriteria = sc.cached ?? []; + let uncachedSortCriteria = sc.uncached ?? [];
1116-1144:⚠️ Potential issue
skipReasonsstill missesAudioChannelkeys → runtime crashLater code increments
skipReasons.excludedAudioChannel/requiredAudioChannel(e.g. lines 1730-1755) but those keys are not initialised here. AccessingskipReasons.excludedAudioChannel.totalwhen the property isundefinedwill throw.excludedAudioTag: { total: 0, details: {} }, requiredAudioTag: { total: 0, details: {} }, + excludedAudioChannel: { total: 0, details: {} }, + requiredAudioChannel: { total: 0, details: {} },
🧹 Nitpick comments (3)
packages/core/src/transformers/stremio.ts (2)
83-95:bingeGroupcan be empty or meaningless
If every identifying attribute is undefined,bingeGroupbecomes either an empty string or justproxied.– neither is useful for grouping.Consider falling back to a hash of the stream URL / infoHash or omit the field when no identifiers are present.
258-268: Subtitle erroridis unsafe
error.${errorTitle}can contain spaces or unicode, violating Stremio’sidexpectations.
Sanitise with a slug generator or at leastencodeURIComponent.packages/core/src/main.ts (1)
1070-1076: RedundantelseafterthrowAfter the
throwinvalidateAddonthe subsequentelse ifchain is unnecessary; execution never reaches it when thethrowhappens. Droppingelseblocks shortens indentation.[Nit] consider:
- } else if ( + } + if ( Env.BASE_URL &&🧰 Tools
🪛 Biome (1.9.4)
[error] 1075-1075: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/core/src/db/schemas.ts(1 hunks)packages/core/src/main.ts(1 hunks)packages/core/src/transformers/stremio.ts(1 hunks)packages/core/src/utils/cache.ts(3 hunks)packages/core/src/utils/env.ts(1 hunks)packages/core/src/utils/http.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/utils/cache.ts
- packages/core/src/utils/env.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/core/src/utils/http.ts (5)
packages/core/src/utils/logger.ts (1)
maskSensitiveInfo(123-128)packages/core/src/utils/cache.ts (1)
Cache(14-126)packages/core/src/wrapper.ts (1)
makeRequest(240-247)packages/core/src/utils/constants.ts (1)
HEADERS_FOR_IP_FORWARDING(910-910)packages/core/src/utils/env.ts (1)
Env(150-1060)
packages/core/src/transformers/stremio.ts (5)
packages/core/src/utils/logger.ts (1)
createLogger(62-110)packages/core/src/db/schemas.ts (13)
UserData(356-356)Resource(103-103)ParsedStream(650-650)AIOStream(652-696)AIOStream(698-698)Subtitle(448-448)SubtitleResponse(447-447)MetaPreview(562-562)CatalogResponse(560-560)Meta(561-561)MetaResponse(559-559)AddonCatalog(573-573)AddonCatalogResponse(572-572)packages/core/src/main.ts (2)
AIOStreamsError(49-52)AIOStreamsResponse(54-58)packages/core/src/formatters/index.ts (1)
createFormatter(17-41)packages/core/src/utils/env.ts (1)
Env(150-1060)
🪛 Biome (1.9.4)
packages/core/src/main.ts
[error] 217-217: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 299-299: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 663-664: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1075-1075: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 2476-2478: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2571-2574: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (6)
packages/core/src/utils/config.ts (2)
33-118: Refactor duplicate credential mapping logic.The
getServiceCredentialDefaultandgetServiceCredentialForcedfunctions contain nearly identical switch statement structures, differing only in the environment variables they access. This violates the DRY principle.Also applies to: 120-205
308-312: Consider separating validation from initialization.The
validateConfigfunction performs validation but also has the side effect of initializingAIOStreams. This violates the single responsibility principle. Consider returning the validated config and performing initialization separately.packages/core/src/db/schemas.ts (4)
4-18:z.enum(constants.X)likely fails at compile-time if the array isn't areadonly tuple.
z.enumexpects a tuple (readonly [...]), not a plainstring[]. Unless every exported constant (SERVICES,RESOLUTIONS, …) is declared asas const, the current code will raise a TypeScript error.Also applies to: 23-24, 29-30
42-47: Plain-text credential fields – consider hashing / encryption.
StreamProxyConfig.credentialsandServiceSchema.credentialsaccept raw strings. Storing service or proxy secrets unhashed inside the user's config/database is risky.Also applies to: 91-95
70-84: Use non-negative validation for size filters.The size filter tuples use
min(0)which allows zero but makes little sense for file sizes. Consider usingmin(1)ornonnegative()for better validation.
358-368: Expand TABLES constant or consider alternative approach.The TABLES constant only defines one table, which suggests this pattern may not be fully implemented.
🧹 Nitpick comments (3)
packages/core/src/proxy/base.ts (3)
55-69: Missing normalisation ofpublicIpwhen set in configWhen
this.config.publicIpis already present you return it immediately, but you never write back the fetched IP later.
Down-stream code that relies onconfig.publicIpwill trigger a fresh network call every time the process restarts, defeating the cache after a cold start.After successfully retrieving the IP you should persist it:
if (publicIp && cache) { cache.set(cacheKey, publicIp, 900); + this.config.publicIp = publicIp;[performance]
70-75: Potential race condition on concurrent look-upsMultiple concurrent
getPublicIp()calls using the samecacheKeycan all miss the cache before the first one stores the value, causing several redundant network requests.Mitigate by memoising the in-flight
Promise, or use the cache to store a sentinel while the request is pending.[performance]
112-131: Return type ambiguity on failure
generateUrlsresolves tonullon any error but[]when input is empty. Callers now have to handle two “no data” cases (nullvs[]). Ifnullmeans “error”, document it explicitly; otherwise return[]consistently.[api-consistency]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/core/src/db/schemas.ts(1 hunks)packages/core/src/proxy/base.ts(1 hunks)packages/core/src/utils/config.ts(1 hunks)
🔇 Additional comments (1)
packages/core/src/proxy/base.ts (1)
87-91:AbortSignal.timeoutlimits runtime compatibility
AbortSignal.timeout()is only available in Node ≥ 18.17 and not in browsers. If the core library is meant to be isomorphic, this call will break in older runtimes.Guard it or fall back to a manual
AbortController+setTimeout.[compatibility]
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.env.sample (2)
292-293: Inline comments break.envparsing – move them above assignments.
Values such asSTATIC_RATE_LIMIT_WINDOW=5 # Time window in seconds. STATIC_RATE_LIMIT_MAX_REQUESTS=100 # Max requests per IP in window.include the trailing comment in the variable value. Relocate these inline comments to separate lines immediately preceding each assignment.
354-354: Inline comments break.envparsing – relocate above assignment.
The inline comment onMEDIAFUSION_CONFIG_TIMEOUT=5000 # Timeout (ms) for /encrypt-user-data endpoint.will be ingested as part of the value. Please extract it to the line above.
🧹 Nitpick comments (4)
.env.sample (4)
362-362: Quoted JSON in environment variable may not parse correctly.
DEFAULT_JACKETTIO_INDEXERS='["bitsearch", "eztv", "thepiratebay", "therarbg", "yts"]'embeds a JSON array in single quotes. Some parsers include quotes or misinterpret commas/escapes. Consider switching to a simple comma-separated list (e.g.,bitsearch,eztv,...)or documenting the exact parser behaviour.
20-20: Clarify thatBASE_URLis required.
This variable is critical for generating installation URLs and self-scraping prevention. Adding a# REQUIREDtag or an example placeholder (e.g.,BASE_URL=https://yourdomain.com) will help users avoid leaving it blank.
10-10: Document expected format forADDON_ID.
To ensure a valid reverse-domain identifier, include an example (e.g.,ADDON_ID=com.yourdomain.myaddon) or reference the naming convention.
47-47: Warn about directory existence for default SQLite URI.
The defaultDATABASE_URI=sqlite://./data/db.sqliteassumes thedatafolder exists and is writable. Consider adding a note to create that directory or adjust the path accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.env.sample(1 hunks)packages/core/src/utils/env.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/utils/env.ts
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (8)
packages/core/src/presets/dmmCast.ts (2)
120-124: 🛠️ Refactor suggestionHarden URL validation
The
endsWith('/manifest.json')check still allows malformed strings likefoo/manifest.jsonfoo. Parse withnew URL()first, then assert the pathname suffix as previously suggested.🧰 Tools
🪛 Biome (1.9.4)
[error] 123-123: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
131-142: 🛠️ Refactor suggestionReplace
thiswith class name inside static methodsUsing
thisinside static methods is flagged by Biome and hurts readability; switch toDMMCastPreset.METADATA/DMMCastPreset.generateAddonas proposed earlier.🧰 Tools
🪛 Biome (1.9.4)
[error] 131-131: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 132-132: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 136-136: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 137-137: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 138-138: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 140-140: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
packages/core/src/main.ts (6)
101-108:⚠️ Potential issueCall
checkInitialised()at the start ofgetStreams
getStreamsstill runs without verifying thatinitialise()has completed, risking crashes on un-set state. Insertthis.checkInitialised();as the very first line of the method.
1116-1145:⚠️ Potential issue
skipReasonsmissing audio-channel entries → runtime crashLater code writes to
skipReasons.excludedAudioChannel/requiredAudioChannel, but these keys aren’t initialised here, causingTypeError. Add both objects alongside the others.
1437-1470:⚠️ Potential issueUnsafe optional-chaining on
.lengthstill unguardedExpressions like
file?.visualTags.lengthdereferencelengthwhen the array may beundefined, throwing at runtime. Usefile?.visualTags?.length ?? 0(or a helper) throughout this block and similar ones for audioTags, audioChannels, languages.
2337-2340:⚠️ Potential issue
sortCriteriadereference can throw
this.userData.sortCriteriais optional yet.global,.cached,.uncachedare accessed without guards. Apply optional-chaining/null-coalescing to preventTypeError.
2113-2119:⚠️ Potential issue
-1treated as truthy in provider comparison
if (aProviderIndex && bProviderIndex && …)considers-1as true, producing wrong ordering when neither provider is configured. Explicitly compare against-1.
2542-2547: 🛠️ Refactor suggestionVariable leakage in
switchcaseservice
const index = …is declared directly in thecasewithout braces, allowing it to leak into other cases and keeping Biome unhappy. Wrap the entire clause in{}.
🧹 Nitpick comments (2)
packages/core/src/parser/regex.ts (2)
105-108: AAC pattern matches too broadly
q?aac(?:[ .\\-_]?2)?will match the string “qaac” (an encoder, not the codec) and any word ending in “aac”. Consider anchoring with word-boundaries and dropping the optional leadingq?unless that was intentional.
56-61: Low-resolution regexes can fire inside larger numbersPatterns for
360p,240p,144pdon’t enforce leading boundaries, so “1360p” will incorrectly satisfy360p. Prefix with a word/start boundary (e.g.\\b(360[pi])\\b) to avoid false positives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/core/src/main.ts(1 hunks)packages/core/src/parser/regex.ts(4 hunks)packages/core/src/presets/dmmCast.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/core/src/parser/regex.ts (1)
packages/core/src/utils/constants.ts (7)
RESOLUTIONS(876-876)QUALITIES(877-877)VISUAL_TAGS(878-878)AUDIO_TAGS(879-879)AUDIO_CHANNELS(880-880)LANGUAGES(885-885)ENCODES(881-881)
🪛 Biome (1.9.4)
packages/core/src/main.ts
[error] 217-217: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 299-299: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 663-664: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1075-1075: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 2473-2475: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 2568-2571: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
packages/core/src/presets/dmmCast.ts
[error] 123-123: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 131-131: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 132-132: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 136-136: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 137-137: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 138-138: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 140-140: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/core/src/utils/constants.ts (1)
87-93: Constructor parameter order is still error-proneStatus code precedes
message, so supplying only a message silently overridesstatusCode. Consider swapping the parameters or accepting an options object, as suggested in the earlier review.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/src/utils/constants.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/core/src/utils/constants.ts (1)
packages/core/src/db/schemas.ts (1)
Option(199-199)
…efined for proxy id
…tus and status text
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
packages/core/src/db/users.ts (2)
152-188: Large commented-out validation block still presentThe 30-line block of commented code was flagged in earlier reviews; it remains unchanged. Please either delete it or reinstate proper validation through
validateConfig/UserDataSchema.safeParseto keep the file concise.🧰 Tools
🪛 Biome (1.9.4)
[error] 152-152: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
261-265: UseCOUNT(*)instead of fetching every rowFetching all users just to count them is O(n) and wastes memory. A single aggregate query is both faster and lighter:
- const result = await db.query('SELECT * FROM users'); - return result.length; + const result = await db.query('SELECT COUNT(*) as cnt FROM users'); + return Number(result[0].cnt);packages/core/src/main.ts (4)
101-108:⚠️ Potential issueAdd initialisation guard in
getStreams()
getStreams()can be invoked beforeinitialise()has run, leaving internal structures empty and crashing downstream logic.
Insert an earlythis.checkInitialised().public async getStreams( id: string, type: string, preCaching: boolean = false ): Promise<AIOStreamsResponse<ParsedStream[]>> { + this.checkInitialised(); logger.info(`Handling stream request`, { type, id });
2337-2340:⚠️ Potential issue
sortCriteriamay beundefined– add defensive defaults
this.userData.sortCriteriais optional, yet the code dereferences it unguarded. A missing user setting will explode here.- let sortCriteria = this.userData.sortCriteria.global; - let cachedSortCriteria = this.userData.sortCriteria.cached; - let uncachedSortCriteria = this.userData.sortCriteria.uncached; + const sc = this.userData.sortCriteria ?? {}; + let sortCriteria = sc.global ?? []; + let cachedSortCriteria = sc.cached ?? []; + let uncachedSortCriteria = sc.uncached ?? [];
2113-2119:⚠️ Potential issueTruthiness check with
-1breaks ordering
aProviderIndex && bProviderIndextreats-1as truthy, so two “not-found” providers pass the branch and incorrectly influence sorting.- if ( - aProviderIndex && - bProviderIndex && - aProviderIndex !== bProviderIndex - ) { + if ( + aProviderIndex !== -1 && + bProviderIndex !== -1 && + aProviderIndex !== bProviderIndex + ) { return aProviderIndex - bProviderIndex; }
1437-1450:⚠️ Potential issueUnsafe
?.lengthdereference – possibleTypeError
file?.visualTags.length(and similar audio/language variants) still accesses.lengthwhen the array isundefined, breaking the filter pipeline.- (file?.visualTags.length ? file.visualTags : ['Unknown']).includes( + ((file?.visualTags?.length ?? 0) > 0 + ? file!.visualTags! + : ['Unknown'] + ).includes( tag )Apply the same double-optional-chain (or a helper like
safeArray) to every.lengthaccess forvisualTags,audioTags,audioChannels, andlanguages..env.sample (1)
356-356: Inline comments break.envparsing
The assignment on line 356 includes an inline comment which will be parsed as part of the value. Move the comment above the variable to keep the value clean.- MEDIAFUSION_CONFIG_TIMEOUT=5000 # Timeout (ms) for /encrypt-user-data endpoint. + # Timeout (ms) for /encrypt-user-data endpoint. + MEDIAFUSION_CONFIG_TIMEOUT=5000
🧹 Nitpick comments (4)
packages/core/src/db/users.ts (2)
1-18: Remove unused importmaskSensitiveInfo
maskSensitiveInfois imported but never referenced, which will trigger unused-variable lint errors and slightly increases bundle size.-import { +import { decryptString, deriveKey, encryptString, generateUUID, getTextHash, - maskSensitiveInfo, createLogger,
369-381: Potential infinite recursion ingenerateUUIDThe method retries up to 10 times, but if the UUID generator is ever faulty or the table extremely full, it returns a generic
USER_ERROR. Consider:
- Raising a dedicated
UUID_CONFLICTerror for clearer diagnostics.- Switching to a database-side uniqueness check (
INSERT ... ON CONFLICT DO NOTHINGand loop) to avoid extra round-trips.🧰 Tools
🪛 Biome (1.9.4)
[error] 375-375: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 378-378: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
.env.sample (2)
8-10: Avoid quoting simple string values
While many dotenv parsers strip quotes, including them can sometimes lead to unexpected literal quotes in the value. Remove quotes forADDON_NAMEandADDON_ID.- ADDON_NAME="AIOStreams" - ADDON_ID="aiostreams.viren070.com" + ADDON_NAME=AIOStreams + ADDON_ID=aiostreams.viren070.com
222-230: Standardise commented defaults vs assignments
Several proxy-related defaults (e.g.DEFAULT_PROXY_ENABLED) remain commented out, while other defaults are assigned. For consistency, list all default settings as active (even if they match the comments), so users can override them directly.- # DEFAULT_PROXY_ENABLED=true # Default state for enabling a stream proxy. + DEFAULT_PROXY_ENABLED=true # Default state for enabling a stream proxy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.env.sample(1 hunks)packages/core/src/db/users.ts(1 hunks)packages/core/src/main.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/core/src/db/users.ts
[error] 25-383: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
[error] 64-64: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 67-67: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 142-142: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 152-152: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 229-229: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 233-233: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 375-375: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 378-378: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
packages/core/src/main.ts
[error] 217-217: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 299-299: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 663-664: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1075-1075: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 2473-2475: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
| // with stremio auth, we are given the encrypted password | ||
| // with api use, we are given the password | ||
| // GET /user should also return | ||
|
|
||
| static async getUser( | ||
| uuid: string, | ||
| password: string | ||
| ): Promise<UserData | null> { | ||
| try { | ||
| const result = await db.query( | ||
| 'SELECT config, config_salt, password_hash FROM users WHERE uuid = ?', | ||
| [uuid] | ||
| ); | ||
|
|
||
| if (!result.length || !result[0].config) { | ||
| return Promise.reject(new APIError(constants.ErrorCode.USER_NOT_FOUND)); | ||
| } | ||
|
|
||
| await db.execute( | ||
| 'UPDATE users SET accessed_at = CURRENT_TIMESTAMP WHERE uuid = ?', | ||
| [uuid] | ||
| ); | ||
|
|
||
| const isValid = await this.verifyUserPassword( | ||
| password, | ||
| result[0].password_hash | ||
| ); | ||
| if (!isValid) { | ||
| return Promise.reject( | ||
| new APIError(constants.ErrorCode.USER_INVALID_PASSWORD) | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Ambiguous password parameter may break authentication
getUser() accepts either the raw password or the previously-returned encrypted password (see comment lines 119-122).
verifyUserPassword() hashes the supplied value and compares it with the stored hash, which only works for a raw password.
Passing the already-encrypted string will always fail verification, resulting in USER_INVALID_PASSWORD.
Consider one of the following:
- static async getUser(uuid: string, password: string)
+ // passRawPassword clarifies the caller’s intent
+ static async getUser(
+ uuid: string,
+ password: string,
+ passRawPassword = true
+ )or expose dedicated methods (getUserWithPassword, getUserWithEncryptedPassword) to avoid overloading a single parameter.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 142-142: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
🤖 Prompt for AI Agents
In packages/core/src/db/users.ts between lines 119 and 151, the getUser method's
password parameter is ambiguous because it can be either a raw or encrypted
password, but verifyUserPassword expects a raw password. To fix this, separate
the logic by creating two distinct methods, such as getUserWithPassword for raw
passwords and getUserWithEncryptedPassword for encrypted ones, ensuring each
method handles verification appropriately without overloading a single
parameter.
| case 'addon': | ||
| // find the first occurence of the stream.addon.id in the addons array | ||
| const idx = userData.presets.findIndex( | ||
| (p) => JSON.stringify(p) === stream.addon.id | ||
| ); | ||
| return multiplier * (idx !== -1 ? -idx : 0); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Wrap case 'addon' in braces to avoid scope leakage
Declaring const idx directly inside the switch case leaks the identifier to subsequent cases and keeps Biome unhappy.
- case 'addon':
- // find the first occurence of the stream.addon.id in the addons array
- const idx = userData.presets.findIndex(
- (p) => JSON.stringify(p) === stream.addon.id
- );
- return multiplier * (idx !== -1 ? -idx : 0);
+ case 'addon': {
+ const idx = userData.presets.findIndex(
+ (p) => JSON.stringify(p) === stream.addon.id
+ );
+ return multiplier * (idx !== -1 ? -idx : 0);
+ }📝 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.
| case 'addon': | |
| // find the first occurence of the stream.addon.id in the addons array | |
| const idx = userData.presets.findIndex( | |
| (p) => JSON.stringify(p) === stream.addon.id | |
| ); | |
| return multiplier * (idx !== -1 ? -idx : 0); | |
| case 'addon': { | |
| // find the first occurence of the stream.addon.id in the addons array | |
| const idx = userData.presets.findIndex( | |
| (p) => JSON.stringify(p) === stream.addon.id | |
| ); | |
| return multiplier * (idx !== -1 ? -idx : 0); | |
| } |
🤖 Prompt for AI Agents
In packages/core/src/main.ts around lines 2452 to 2458, the declaration of const
idx inside the 'addon' case of the switch statement causes scope leakage to
other cases. To fix this, wrap the entire 'addon' case block in braces { } to
create a new block scope, ensuring idx is scoped only within this case and
preventing any leakage.
| # Set the verbosity of logs. Options: "error", "warn", "info", "http", "verbose","debug", "silly" | ||
| # Default: info | ||
| LOG_LEVEL=http | ||
|
|
There was a problem hiding this comment.
Update default log level documentation
The comment indicates the default is info, but LOG_LEVEL is set to http. Align the documentation with the actual setting.
- # Default: info
+ # Default: http📝 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.
| # Set the verbosity of logs. Options: "error", "warn", "info", "http", "verbose","debug", "silly" | |
| # Default: info | |
| LOG_LEVEL=http | |
| # Set the verbosity of logs. Options: "error", "warn", "info", "http", "verbose","debug", "silly" | |
| # Default: http | |
| LOG_LEVEL=http |
🤖 Prompt for AI Agents
In the .env.sample file around lines 181 to 184, the comment states the default
log level is "info" but the LOG_LEVEL is set to "http". Update the comment to
reflect that the default log level is "http" to match the actual setting in the
file.
| # Comma-separated list of trusted UUIDs. | ||
| # Trusted users can access features like regex filters if REGEX_FILTER_ACCESS is 'trusted'. | ||
| # Example: TRUSTED_UUIDS=ae32f456-1234-5678-9012-345678901234,another-uuid-here | ||
| # TRUSTED_UUIDS= | ||
|
|
||
| # --- Regex Filter Access --- |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Uncomment TRUSTED_UUIDS assignment
TRUSTED_UUIDS is currently commented out, so users won’t see the variable to populate. Provide an explicit blank assignment for clarity.
- # TRUSTED_UUIDS=
+ TRUSTED_UUIDS=📝 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.
| # Comma-separated list of trusted UUIDs. | |
| # Trusted users can access features like regex filters if REGEX_FILTER_ACCESS is 'trusted'. | |
| # Example: TRUSTED_UUIDS=ae32f456-1234-5678-9012-345678901234,another-uuid-here | |
| # TRUSTED_UUIDS= | |
| # --- Regex Filter Access --- | |
| # Comma-separated list of trusted UUIDs. | |
| # Trusted users can access features like regex filters if REGEX_FILTER_ACCESS is 'trusted'. | |
| # Example: TRUSTED_UUIDS=ae32f456-1234-5678-9012-345678901234,another-uuid-here | |
| TRUSTED_UUIDS= | |
| # --- Regex Filter Access --- |
🤖 Prompt for AI Agents
In .env.sample around lines 126 to 131, the TRUSTED_UUIDS variable is commented
out, which may confuse users about where to set trusted UUIDs. Uncomment the
TRUSTED_UUIDS line and assign it an empty value to explicitly show users where
to input their trusted UUIDs.
…ration, correctly handle undefined value in tryDecrypt, only decrypt when defined
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (10)
packages/core/src/db/users.ts (2)
262-266: Fetching every row to count users remains un-optimised (see prior review).
Please switch toSELECT COUNT(*) AS count …as already suggested – it avoids a full table read.
159-185: Large commented-out validation block still clutters the file
Either re-enable the logic or delete the block to keep the codebase clean.packages/core/src/utils/config.ts (2)
33-118: Duplicate 170-line switch can be collapsed into one helper
getServiceCredentialDefaultandgetServiceCredentialForcedare identical apart from the env-prefix – the DRY violation was flagged previously and is still present. Extract a singlegetServiceCredential(serviceId, credentialId, prefix)helper to remove the duplication.Also applies to: 120-205
308-312:validateConfigstill initialisesAIOStreams– breaks SRPValidation should not have the side-effect of spinning up the streaming engine. Move the
AIOStreamsinitialisation to the caller after successful validation to keep responsibilities separated.packages/core/src/main.ts (6)
2337-2340: PossibleTypeErrorwhensortCriteriais undefined
this.userData.sortCriteriais optional, yet the code directly
dereferences.global,.cached,.uncached.- let sortCriteria = this.userData.sortCriteria.global; - let cachedSortCriteria = this.userData.sortCriteria.cached; - let uncachedSortCriteria = this.userData.sortCriteria.uncached; + const sc = this.userData.sortCriteria ?? {}; + let sortCriteria = sc.global ?? []; + let cachedSortCriteria = sc.cached ?? []; + let uncachedSortCriteria = sc.uncached ?? [];
2104-2119: Truthiness check still mis-orders providers
aProviderIndex && bProviderIndex && …considers-1“truthy”, so when both
indices are-1the branch executes, giving unpredictable order.- if (aProviderIndex && bProviderIndex && aProviderIndex !== bProviderIndex) { + if ( + aProviderIndex !== -1 && + bProviderIndex !== -1 && + aProviderIndex !== bProviderIndex + ) { return aProviderIndex - bProviderIndex; }
1436-1450:⚠️ Potential issueUnsafe optional-chaining on
.length– still throws
file?.visualTags.lengthdereferenceslengtheven when
visualTagsisundefined, producingTypeError.
This was flagged earlier but is still present in several places (visualTags,
audioTags, audioChannels, languages).- (file?.visualTags.length ? file.visualTags : ['Unknown']).includes(tag) + ((file?.visualTags?.length ?? 0) > 0 + ? file!.visualTags! + : ['Unknown'] + ).includes(tag)Please apply the same pattern to every
?.<prop>.lengthoccurrence.
101-109:⚠️ Potential issueCall
checkInitialised()at the start ofgetStreams
getStreams()is publicly exposed but still assumes thatinitialise()has been invoked. A single forgotten call will cause a hard-to-diagnose runtime failure deep in the pipeline.
Add the guard that was requested in previous reviews:public async getStreams( id: string, type: string, preCaching: boolean = false ): Promise<AIOStreamsResponse<ParsedStream[]>> { + this.checkInitialised(); logger.info(`Handling stream request`, { type, id });
2470-2490: 🛠️ Refactor suggestionWrap
case 'visualTag'in braces to avoid scope leakage
let minIndexleaks into subsequentcaseclauses and keeps Biome unhappy
(noSwitchDeclarations).- case 'visualTag': - let minIndex = userData.preferredVisualTags?.length; + case 'visualTag': { + let minIndex = userData.preferredVisualTags?.length; … - return multiplier * -minIndex; + return multiplier * -minIndex; + }Apply the same pattern to other
caseblocks with local declarations
(e.g.audioTag,language).🧰 Tools
🪛 Biome (1.9.4)
[error] 2473-2475: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
1116-1145:⚠️ Potential issue
skipReasonsstill missesexcludedAudioChannel/requiredAudioChannelThe filter logic later updates
skipReasons.excludedAudioChanneland
skipReasons.requiredAudioChannel(~1724-1755), but the keys are not
initialised here, soskipReasons.excludedAudioChannel.totalcrashes with
TypeError: Cannot read properties of undefined.excludedAudioTag: { total: 0, details: {} }, requiredAudioTag: { total: 0, details: {} }, + excludedAudioChannel: { total: 0, details: {} }, + requiredAudioChannel: { total: 0, details: {} }, excludedLanguage: { total: 0, details: {} },
🧹 Nitpick comments (2)
packages/core/src/db/users.ts (2)
25-31: Static-only class – consider converting to a plain module
UserRepositoryonly exposes static members. Replacing it with named exports (or a namespace) trims boilerplate and avoids the recurring “thisin static context” lints flagged by Biome.🧰 Tools
🪛 Biome (1.9.4)
[error] 25-384: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
65-65: Use explicit class name instead ofthisin static contextTo silence Biome’s
noThisInStaticwarning and improve clarity:-const uuid = await this.generateUUID(); +const uuid = await UserRepository.generateUUID();Do the same for other static self-calls.
Also applies to: 82-83, 376-379
🧰 Tools
🪛 Biome (1.9.4)
[error] 65-65: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/core/src/db/users.ts(1 hunks)packages/core/src/main.ts(1 hunks)packages/core/src/utils/config.ts(1 hunks)packages/core/src/utils/crypto.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/utils/crypto.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/core/src/db/users.ts
[error] 25-384: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
[error] 65-65: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 68-68: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 143-143: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 153-153: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 230-230: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 234-234: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 376-376: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 379-379: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
packages/core/src/main.ts
[error] 217-217: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 299-299: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 663-664: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 853-854: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1075-1075: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 2473-2475: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
closes #176 ✅
closes #174 ✅
closes #158 ✅
closes #141 😐
closes #53 ✅
closes #126 😐
closes #32 😐
closes #64 ✅
closes #116 ✅
closes #114 ✅
closes #101 ✅
closes #46 ✅
closes #96 ✅
closes #95 ✅
closes #90 ✅
closes #55 ✅
closes #73 ✅
closes #111 ✅
closes #182 ✅
closes #180 ✅
closes #143 ✅
closes #167 ✅
closes #60✅
closes #11 ✅
closes #153 ✅
closes #113 ✅
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores