feat: Introduce dependency injection core interfaces and utilities #176
Conversation
📝 WalkthroughWalkthroughA new standalone dependency injection library ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
libs/sdk/src/**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
libs/**⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (4)📚 Learning: 2025-12-19T02:04:46.475ZApplied to files:
📚 Learning: 2025-12-19T02:04:46.475ZApplied to files:
📚 Learning: 2025-12-19T02:04:46.475ZApplied to files:
📚 Learning: 2025-12-19T02:04:46.475ZApplied to files:
🔇 Additional comments (3)
Comment |
|
| Status | Scanner |  Critical |
 High |
 Medium |
 Low |
Total (0) |
|---|---|---|---|---|---|---|
| Open Source Security | 0 | 0 | 0 | 0 | See details | |
| ✅ | Code Security | 0 | 0 | 0 | 0 | 0 issues |
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/sdk/src/common/metadata/provider.metadata.ts (2)
9-16: Eliminate schema duplication with DI library.The
frontMcpProviderMetadataSchemais nearly identical toproviderMetadataSchemadefined inlibs/di/src/metadata/provider.schema.ts. Both define the same fields with the same constraints.Consider either:
- Importing and using
providerMetadataSchemafrom@frontmcp/diif the schemas are meant to be identical- Extending the base DI schema if SDK needs additional validation
- Documenting why separate schemas are needed if there's a specific reason
🔎 Option 1: Import from DI library
-import { ProviderScope, ProviderMetadata } from '@frontmcp/di'; +import { ProviderScope, ProviderMetadata, providerMetadataSchema } from '@frontmcp/di'; import { RawZodShape } from '../types'; /** * FrontMCP-specific schema for provider metadata validation. * Uses the base ProviderMetadata interface from DI. */ -export const frontMcpProviderMetadataSchema = z - .object({ - id: z.string().optional(), - name: z.string().min(1), - description: z.string().optional(), - scope: z.nativeEnum(ProviderScope).optional().default(ProviderScope.GLOBAL), - } satisfies RawZodShape<ProviderMetadata>) - .passthrough(); +export const frontMcpProviderMetadataSchema = providerMetadataSchema;
9-16: Use.strict()instead of.passthrough()per coding guidelines.The schema uses
.passthrough()which allows additional properties not defined in the schema. According to the coding guidelines: "Use.strict()on all Zod schemas for validation."As per coding guidelines, learnings indicate all Zod schemas should use
.strict()for validation.🔎 Proposed fix
export const frontMcpProviderMetadataSchema = z .object({ id: z.string().optional(), name: z.string().min(1), description: z.string().optional(), scope: z.nativeEnum(ProviderScope).optional().default(ProviderScope.GLOBAL), } satisfies RawZodShape<ProviderMetadata>) - .passthrough(); + .strict();
🧹 Nitpick comments (25)
libs/sdk/src/logger/logger.utils.ts (1)
19-27: Replaceanywithunknownfor strict type safety.The function parameter and type assertion use
any, which violates the coding guideline requiring strict TypeScript mode. Useunknowninstead and apply proper type narrowing.As per coding guidelines: "Use strict TypeScript mode with no
anytypes without strong justification - useunknowninstead for generic type defaults"🔎 Proposed refactor using `unknown`
-export function normalizeLogger(item: any): LoggerRecord { +export function normalizeLogger(item: unknown): LoggerRecord { if (isClass(item)) { const metadata = collectLoggerMetadata(item); return { kind: LoggerKind.CLASS_TOKEN, provide: item as Type<LogTransportInterface>, metadata }; } - const name = (item as any)?.name ?? String(item); + const name = typeof item === 'object' && item !== null && 'name' in item + ? String((item as { name: unknown }).name) + : String(item); throw new Error(`Invalid app '${name}'. Expected a class or remote app config object.`); }libs/di/tsconfig.spec.json (1)
1-10: Consider using modern moduleResolution strategy.The test configuration is correct and follows standard Jest patterns. However,
moduleResolution: "node10"is an older resolution strategy.💡 Optional: Modernize moduleResolution
Consider using a more modern resolution strategy if it aligns with other library test configurations in the workspace:
{ "extends": "./tsconfig.json", "compilerOptions": { "outDir": "../../dist/out-tsc", "module": "commonjs", - "moduleResolution": "node10", + "moduleResolution": "node", "types": ["jest", "node"] }, "include": ["jest.config.ts", "src/**/*.test.ts", "src/**/*.spec.ts", "src/**/*.d.ts"] }Note: Only apply this if other library test configs use "node" for consistency. If the workspace standardizes on "node10" for Jest compatibility, keep as-is.
libs/sdk/src/adapter/adapter.utils.ts (1)
17-23: Consider usingunknowninstead ofanyfor stricter type safety.The
as anycast bypasses type checking. Per coding guidelines, preferunknownwith proper type narrowing or define a specific interface for the expected shape.🔎 Proposed type-safe approach
- if (item && typeof item === 'object') { - const { provide, useClass, useValue, useFactory, inject } = item as any; + if (item && typeof item === 'object') { + const obj = item as Record<string, unknown>; + const { provide, useClass, useValue, useFactory, inject } = obj;Then add type guards before using each property.
libs/di/jest.config.ts (1)
36-43: Consider raising the branch coverage threshold before stabilization.The 55% branch threshold is notably lower than statements (70%) and functions (90%). While the comment explains this is for initial release, container logic with "many edge case branches" is precisely where untested branches could hide bugs.
Consider setting a timeline to raise this threshold (e.g., to 70%) before the library is marked stable, or adding a tracking issue.
libs/di/src/utils/metadata.utils.ts (1)
20-22: Consider usingunknownas the default generic type.The
anydefault forTpropagates untyped values. Usingunknownas the default would encourage callers to specify their expected type.🔎 Proposed change
-export function getMetadata<T = any>(key: any, target: any, propertyKey?: string | symbol): T | undefined { +export function getMetadata<T = unknown>(key: any, target: any, propertyKey?: string | symbol): T | undefined {The
keyandtargetparameters usinganyare acceptable since they mirror thereflect-metadataAPI signature.libs/di/src/interfaces/registry.interface.ts (2)
44-47: Move inline import to top-level imports for consistency.The inline
import('../metadata/provider.metadata.js').ProviderScopeis inconsistent with the top-level import style used elsewhere in this file.🔎 Proposed fix
import type { Token } from './base.interface.js'; import type { ProviderRecord } from '../records/provider.record.js'; +import type { ProviderScope } from '../metadata/provider.metadata.js'; // ... - getProviderScope(rec: ProviderRecord): import('../metadata/provider.metadata.js').ProviderScope; + getProviderScope(rec: ProviderRecord): ProviderScope;
64-76: Consider adding type parameters to Token for stronger typing.The
DiViewsinterface uses untypedTokenwhich loses type information. UsingToken<unknown>explicitly documents the intent.🔎 Proposed improvement
export interface DiViews { - global: ReadonlyMap<Token, unknown>; - context: Map<Token, unknown>; + global: ReadonlyMap<Token<unknown>, unknown>; + context: Map<Token<unknown>, unknown>; }libs/di/src/registry/indexed.types.ts (1)
38-53: Consider adding type parameter to Token for consistency.Similar to other interfaces,
IndexedEntry<T>uses untypedTokenon line 40. While theinstance: Tprovides typing for the value, the token itself loses its type association.🔎 Proposed change
export interface IndexedEntry<T> { - /** Token for this entry */ - token: Token; + /** Token for this entry */ + token: Token<T>;This ties the token's type parameter to the instance type.
libs/sdk/src/common/records/logger.record.ts (1)
15-15: Good simplification of the type alias.The
LoggerRecordtype has been correctly simplified from union syntax (| LoggerClassToken) to a direct alias. Since there was only one type in the union, this improves readability without changing behavior.libs/sdk/src/common/interfaces/provider.interface.ts (1)
4-4: Consider static analysis suggestion for empty interface.Biome flags the empty
ProviderInterfaceas equivalent to{}and suggests using a type alias instead. However, keeping it as an interface may be intentional to allow future extension or to serve as a marker type.If the interface is purely a marker with no planned properties, consider:
export type ProviderInterface = Record<string, never>;Otherwise, the current empty interface is acceptable for future extensibility.
Based on coding guidelines: As per static analysis tools, empty interfaces are flagged by Biome.
libs/di/src/__tests__/di.constants.spec.ts (1)
19-21: Consider making symbol description assertions more flexible.The test asserts exact string match on
META_ASYNC_WITH.toString()(line 20). Symbol descriptions are implementation details; if the symbol creation changes slightly (e.g., different spacing or naming), this test will break. Consider testing only that it's a symbol, or use a regex pattern for more flexibility.🔎 Alternative approach using pattern matching
- it('should have descriptive name', () => { - expect(META_ASYNC_WITH.toString()).toBe('Symbol(di:async-with)'); - }); + it('should have descriptive name', () => { + expect(META_ASYNC_WITH.toString()).toMatch(/Symbol\(.*async.*with.*\)/i); + });Similarly for META_ASYNC_WITH_TOKENS at line 30.
libs/di/src/__tests__/metadata.utils.spec.ts (1)
10-47: Consider adding error case tests for robustness.The test coverage for
getMetadatais good for happy paths, but lacks error case testing (e.g., null/undefined targets, invalid metadata keys). While these utilities are thin wrappers overreflect-metadata, verifying error handling ensures the API behaves predictably under invalid conditions.Based on learnings, test all code paths including error cases.
🔎 Suggested additional test cases
it('should handle null target gracefully', () => { const key = Symbol('test-key'); expect(() => getMetadata(key, null as any)).not.toThrow(); }); it('should handle undefined target gracefully', () => { const key = Symbol('test-key'); expect(() => getMetadata(key, undefined as any)).not.toThrow(); });libs/sdk/src/common/interfaces/resource.interface.ts (1)
18-23: Consider stricter typing or add justification foranyusage.The
FunctionResourceTypeusesanyfor both parameters and return type, which is permissive. Per coding guidelines, preferunknownoveranywithout strong justification.Consider either:
- Use
unknownfor stricter type safety:(...args: unknown[]) => unknown- Add a JSDoc comment explaining why
anyis necessary for this dynamic builder pattern🔎 Proposed refactor with stricter types
/** * Function-style resource type. * This represents resources created via resource() or resourceTemplate() builders. * The function returns a handler that will be invoked for the resource. + * Note: Uses 'unknown' instead of 'any' for type safety. */ -export type FunctionResourceType = (...args: any[]) => any; +export type FunctionResourceType = (...args: unknown[]) => unknown;libs/sdk/src/common/records/adapter.record.ts (1)
13-13: Minor: Inconsistent property delimiter.Line 13 uses a semicolon as the property delimiter, while most TypeScript interfaces conventionally use commas or no delimiters. Consider using commas for consistency with typical interface style.
🔎 Suggested fix for consistency
export interface AdapterClassTokenRecord { kind: AdapterKind.CLASS_TOKEN; - provide: Type; + provide: Type, metadata: AdapterMetadata; }libs/sdk/src/common/interfaces/scope.interface.ts (1)
3-3: Consider adding members toScopeInterfaceor converting to a type alias.An empty interface is equivalent to
{}and may not provide meaningful type safety. If this is intended as a marker interface for structural typing, consider adding a discriminant property or using a branded type instead.🔎 Alternative approaches
Option 1: Add a discriminant property
-export interface ScopeInterface {} +export interface ScopeInterface { + readonly __scopeBrand: unique symbol; +}Option 2: Use a type alias with branding
-export interface ScopeInterface {} +export type ScopeInterface = { readonly __scopeBrand: unique symbol };Option 3: If truly meant to accept any object
-export interface ScopeInterface {} +export type ScopeInterface = Record<string, unknown>;Based on static analysis, this addresses Biome lint rule
lint/suspicious/noEmptyInterface.libs/di/src/__tests__/indexed.registry.spec.ts (1)
17-22: Consider adding error case tests.The test interface uses
anyfor thedatafield, which is acceptable in test code. However, per coding guidelines, consider adding tests for error cases such as invalid record inputs or constructor validation failures to achieve comprehensive coverage.libs/di/src/interfaces/provider.interface.ts (1)
66-72: Consider usingunknown[]instead ofany[]for thedepsparameter.Per coding guidelines,
unknownshould be preferred overanyfor generic type defaults. Thewithmethod'sdepsparameter usesany[], which bypasses type safety.Suggested fix
export interface AsyncProvider<T = unknown> { /** * Static async factory method. * Called instead of constructor when @AsyncWith decorator is used. */ - with?(...deps: any[]): T | Promise<T>; + with?(...deps: unknown[]): T | Promise<T>; }libs/di/src/records/provider.record.ts (1)
58-75: Consider usingunknownfor thevalueand factory return types.Per coding guidelines,
unknownis preferred overany. While these are internal records and type erasure occurs during normalization, usingunknownwould enforce explicit type assertions at consumption sites, improving type safety.Suggested changes
export interface ProviderFactoryRecord { kind: ProviderKind.FACTORY; provide: Token; inject: () => readonly Token[]; - useFactory: (...args: any[]) => any | Promise<any>; + useFactory: (...args: unknown[]) => unknown | Promise<unknown>; metadata: ProviderMetadata; } export interface ProviderInjectedRecord { kind: ProviderKind.INJECTED; provide: Token; - value: any; + value: unknown; metadata: ProviderMetadata; }libs/di/src/registry/registry.base.ts (1)
68-89: Consider using a type guard or interface for theaddRegistrycheck.The duck-typing check at line 70 uses an
anycast. While functional, defining an interface for registries that supportaddRegistrywould improve type safety and documentation.Suggested improvement
interface RegistryHost { addRegistry(name: RegistryKind, registry: unknown): void; } function isRegistryHost(obj: unknown): obj is RegistryHost { return obj !== null && typeof obj === 'object' && typeof (obj as any).addRegistry === 'function'; } // In constructor: if (isRegistryHost(providers)) { providers.addRegistry(name, this); }libs/di/src/utils/provider.utils.ts (1)
150-197: Extract the shared provider dependency logic into a parameterized helper function.Both
providerDiscoveryDepsandproviderInvocationTokenshave identical structural logic—the same switch-case handling for eachProviderKind—but differ in thephaseparameter passed todepsOfClassFn('discovery'vs'invocation'). Additionally,providerDiscoveryDepshas an unusedlocalTokensparameter. Extract a shared helper that accepts phase as a parameter to reduce duplication:function providerTokens( rec: ProviderRecord, phase: 'discovery' | 'invocation', depsOfClassFn: (klass: Type, phase: 'discovery' | 'invocation') => Type[], ): Token[] { // shared switch logic }Then have both functions delegate to it.
libs/di/src/utils/token.utils.ts (1)
64-86: TheforWhat/phaseparameters are not used to differentiate behavior.
readWithParamTypes: TheforWhatparameter appears only in a single error message on line 73; all other logic is identical.depsOfClass: Thephaseparameter is passed through toreadWithParamTypesbut not used in the constructor dependency logic (lines 102–110).depsOfFunc: Thephaseparameter is entirely unused (lines 125–130).Since these are public APIs, removing the parameters would be a breaking change. However, if phase differentiation was intended, the logic should be implemented; otherwise, consider documenting why the parameters are needed or removing them in a future major version.
libs/di/src/registry/indexed.registry.ts (2)
127-146: Consider adding guard for duplicate child adoption.If
adoptFromChildis called twice with the same child, it will create duplicate subscriptions without cleaning up the previous one. Consider adding a guard or cleanup:🔎 Proposed fix
adoptFromChild(child: IndexedRegistry<TInstance, TRecord, TIndexed, any, any>, childOwner: EntryOwnerRef): void { + // Clean up existing subscription if re-adopting same child + const existingUnsub = this.childSubscriptions.get(child); + if (existingUnsub) existingUnsub(); + // Get child's entries and relineage them const childRows = child.listAllIndexed();
265-272: Avoid non-null assertion for filter callback.Per coding guidelines, prefer avoiding
!assertions. While the guard at line 266 makes this safe, TypeScript can infer this with a minor refactor:🔎 Proposed fix
return this.emitter.on((e) => { - if (opts.filter) { - const filtered = e.snapshot.filter((row) => opts.filter!(row.instance)); + const { filter } = opts; + if (filter) { + const filtered = e.snapshot.filter((row) => filter(row.instance)); callback({ ...e, snapshot: filtered }); } else { callback(e); } });libs/di/src/registry/container.ts (2)
153-170: Unnecessaryas anycast in buildMap.Since
auto = falseis passed to super (line 99), andbuildGraph()is called at line 108 afterthis.normalizeProvideris assigned (line 101), the fallback cast at line 159 should never be needed. Consider removing the defensive cast or adding a comment explaining when it would apply.🔎 Proposed fix
protected override buildMap(list: ProviderType[]): RegistryBuildMapResult<ProviderRecord> { const tokens = new Set<Token>(); const defs = new Map<Token, ProviderRecord>(); const graph = new Map<Token, Set<Token>>(); - // Use the normalizer stored on this or fall back to basic - const normalize = (this as any).normalizeProvider ?? createBasicNormalizer(); + const normalize = this.normalizeProvider; for (const raw of list) {
356-401: Consider extracting common instantiation logic.The
hasAsyncWith/initpattern is duplicated acrossinstantiateOne,buildIntoStore, andbuildIntoStoreWithViews. A helper method could reduce duplication:🔎 Sketch of potential helper
private async instantiateClass( klass: Ctor<any>, deps: any[], label: string ): Promise<any> { if (hasAsyncWith(klass)) { const out = (klass as any).with(...deps); return isPromise(out) ? await this.withTimeout(out, this.asyncTimeoutMs, `${label}.with(...)`) : out; } const instance = new klass(...deps); const init = (instance as any)?.init; if (typeof init === 'function') { const ret = init.call(instance); if (isPromise(ret)) await this.withTimeout(ret, this.asyncTimeoutMs, `${label}.init()`); } return instance; }
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
libs/di/src/registry/container.ts (2)
526-565: LGTM! Past issue resolved.The
resolve()method now properly handles asyncinit()by throwing a clear error (lines 551-557), addressing the concern from previous reviews. The error message correctly guides users to useget()afterinitialize()instead.
770-822: LGTM! Past issue resolved.The
createBasicNormalizernow validates that theprovidefield exists foruseValue(lines 784-786),useFactory(lines 796-798), anduseClass(lines 809-811) providers, addressing the concern from previous reviews.libs/di/src/__tests__/container.spec.ts (1)
1192-1302: LGTM! Past issue resolved.Line 1193 now properly uses
it.todo()to mark the unimplemented test, addressing the concern from previous reviews. The remaining tests in this section provide good coverage of edge cases and error handling.
🧹 Nitpick comments (3)
libs/di/src/registry/container.ts (3)
153-170: Remove unnecessaryanycast.Line 159 uses
(this as any).normalizeProviderwith a fallback tocreateBasicNormalizer(), but the constructor always setsthis.normalizeProvider. The cast and fallback are unnecessary.🔎 Proposed refactor
protected override buildMap(list: ProviderType[]): RegistryBuildMapResult<ProviderRecord> { const tokens = new Set<Token>(); const defs = new Map<Token, ProviderRecord>(); const graph = new Map<Token, Set<Token>>(); - // Use the normalizer stored on this or fall back to basic - const normalize = (this as any).normalizeProvider ?? createBasicNormalizer(); + const normalize = this.normalizeProvider; for (const raw of list) {Based on coding guidelines.
335-402: Consider type guards to avoidanycasts.Lines 359, 382, and throughout this method use
(rec as any)to access provider-specific properties. While functional, consider introducing type guards or narrowing functions to avoidanycasts.Example pattern:
function isClassProvider(rec: ProviderRecord): rec is ClassProviderRecord { return rec.kind === ProviderKind.CLASS; }Then use:
if (isClassProvider(rec)) { const klass = rec.useClass; // properly typed }Based on coding guidelines.
268-280: Use specific timer type instead ofany.Line 271 declares
timerasany. Use a more specific type likeReturnType<typeof setTimeout>orNodeJS.Timeoutdepending on your environment.🔎 Proposed refactor
private async withTimeout<T>(p: Promise<T>, ms: number, label: string): Promise<T> { if (ms <= 0 || !Number.isFinite(ms)) return p; - let timer: any; + let timer: ReturnType<typeof setTimeout>; try {Based on coding guidelines.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
libs/di/src/__tests__/async-with.decorator.spec.tslibs/di/src/__tests__/container.spec.tslibs/di/src/__tests__/provider.metadata.spec.tslibs/di/src/interfaces/base.interface.tslibs/di/src/registry/container.tslibs/sdk/src/common/decorators/prompt.decorator.tslibs/sdk/src/common/dynamic/dynamic.plugin.tslibs/sdk/src/common/interfaces/base.interface.tslibs/sdk/src/common/interfaces/prompt.interface.tslibs/sdk/src/common/metadata/flow.metadata.tslibs/sdk/src/common/metadata/provider.metadata.tslibs/sdk/src/common/records/provider.record.tslibs/sdk/src/flows/flow.utils.tslibs/sdk/src/utils/metadata.utils.tslibs/sdk/src/utils/token.utils.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- libs/di/src/tests/provider.metadata.spec.ts
- libs/sdk/src/flows/flow.utils.ts
- libs/sdk/src/common/metadata/flow.metadata.ts
- libs/sdk/src/common/dynamic/dynamic.plugin.ts
- libs/di/src/interfaces/base.interface.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use strict TypeScript mode with noanytypes without strong justification - useunknowninstead for generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific errors instead
Use type parameters with constraints instead of unconstrained generics
Do not mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/common/metadata/provider.metadata.tslibs/di/src/__tests__/container.spec.tslibs/sdk/src/utils/token.utils.tslibs/sdk/src/common/interfaces/prompt.interface.tslibs/di/src/__tests__/async-with.decorator.spec.tslibs/sdk/src/common/records/provider.record.tslibs/sdk/src/common/decorators/prompt.decorator.tslibs/sdk/src/utils/metadata.utils.tslibs/di/src/registry/container.tslibs/sdk/src/common/interfaces/base.interface.ts
libs/sdk/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/src/**/*.ts: MCP response types should use strict MCP protocol types (GetPromptResult, ReadResourceResult) instead ofunknown
Use specific error classes with MCP error codes instead of generic errors
UsegetCapabilities()method for dynamic capability exposure instead of hardcoding capabilities
UsechangeScopeproperty instead ofscopein change events to avoid confusion with Scope class
Validate URIs per RFC 3986 at metadata level using proper validation
Validate hook flows and fail fast on invalid hook configurations with specific error messages
Files:
libs/sdk/src/common/metadata/provider.metadata.tslibs/sdk/src/utils/token.utils.tslibs/sdk/src/common/interfaces/prompt.interface.tslibs/sdk/src/common/records/provider.record.tslibs/sdk/src/common/decorators/prompt.decorator.tslibs/sdk/src/utils/metadata.utils.tslibs/sdk/src/common/interfaces/base.interface.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/common/metadata/provider.metadata.tslibs/di/src/__tests__/container.spec.tslibs/sdk/src/utils/token.utils.tslibs/sdk/src/common/interfaces/prompt.interface.tslibs/di/src/__tests__/async-with.decorator.spec.tslibs/sdk/src/common/records/provider.record.tslibs/sdk/src/common/decorators/prompt.decorator.tslibs/sdk/src/utils/metadata.utils.tslibs/di/src/registry/container.tslibs/sdk/src/common/interfaces/base.interface.ts
libs/sdk/src/common/records/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Centralize record types in common/records directory and import from there instead of module-specific files
Files:
libs/sdk/src/common/records/provider.record.ts
🧠 Learnings (14)
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/*.ts : Use `changeScope` property instead of `scope` in change events to avoid confusion with Scope class
Applied to files:
libs/sdk/src/common/metadata/provider.metadata.tslibs/sdk/src/utils/token.utils.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: FrontMCP is a TypeScript-first schema validation framework - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/common/metadata/provider.metadata.tslibs/sdk/src/common/interfaces/prompt.interface.tslibs/sdk/src/common/decorators/prompt.decorator.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to **/*.test.{ts,tsx} : Test all code paths including error cases and constructor validation
Applied to files:
libs/di/src/__tests__/container.spec.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{adapters,theme,build}/**/*.test.{ts,tsx,js,jsx} : Test component behavior across all platform configurations (OpenAI, Claude, etc.)
Applied to files:
libs/di/src/__tests__/container.spec.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/index.ts : Export public API through barrel files - export users' needed items, no legacy exports with different names
Applied to files:
libs/sdk/src/utils/token.utils.tslibs/sdk/src/utils/metadata.utils.tslibs/sdk/src/common/interfaces/base.interface.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/src/**/*.{ts,tsx} : Never import React-free utilities from frontmcp/ui; use frontmcp/uipack for bundling, build tools, platform adapters, and theme utilities
Applied to files:
libs/sdk/src/utils/token.utils.tslibs/sdk/src/common/decorators/prompt.decorator.tslibs/sdk/src/utils/metadata.utils.tslibs/sdk/src/common/interfaces/base.interface.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/*/index.ts : Implement entry point re-exports in index.ts files for each module (adapters, build, bundler, etc.)
Applied to files:
libs/sdk/src/utils/token.utils.tslibs/sdk/src/common/interfaces/base.interface.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/src/bundler/**/*.{ts,tsx} : The bundler module must re-export utilities from frontmcp/uipack/bundler and provide SSR component bundling functionality
Applied to files:
libs/sdk/src/utils/token.utils.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx,js,jsx} : No React dependency in frontmcp/uipack - use frontmcp/ui for React components and hooks
Applied to files:
libs/sdk/src/utils/token.utils.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/*.ts : Use `getCapabilities()` method for dynamic capability exposure instead of hardcoding capabilities
Applied to files:
libs/sdk/src/utils/token.utils.tslibs/sdk/src/utils/metadata.utils.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/common/records/**/*.ts : Centralize record types in common/records directory and import from there instead of module-specific files
Applied to files:
libs/sdk/src/utils/token.utils.tslibs/sdk/src/common/records/provider.record.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/*.ts : MCP response types should use strict MCP protocol types (GetPromptResult, ReadResourceResult) instead of `unknown`
Applied to files:
libs/sdk/src/common/interfaces/prompt.interface.tslibs/sdk/src/common/decorators/prompt.decorator.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/*.ts : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
libs/sdk/src/common/decorators/prompt.decorator.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
libs/di/src/registry/container.ts
🧬 Code graph analysis (3)
libs/di/src/__tests__/container.spec.ts (1)
libs/di/src/registry/container.ts (2)
DiContainer(73-765)DiContainerOptions(42-54)
libs/sdk/src/common/interfaces/prompt.interface.ts (1)
libs/di/src/interfaces/base.interface.ts (2)
Type(13-15)FuncType(21-21)
libs/sdk/src/common/decorators/prompt.decorator.ts (1)
libs/sdk/src/common/metadata/prompt.metadata.ts (1)
frontMcpPromptMetadataSchema(79-85)
⏰ 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: Lint & Format Checks
- GitHub Check: Build Libraries
🔇 Additional comments (17)
libs/sdk/src/common/decorators/prompt.decorator.ts (4)
2-3: LGTM!Clean import consolidation aligning with the DI refactoring.
27-32: LGTM!Good use of
unknownfor the symbol index type, which follows the coding guidelines. The intersection type correctly models a callable with attached symbol-keyed metadata.
34-47: LGTM!The type assertion on line 41 is justified since the symbol-indexed properties are assigned immediately after via
Object.assign. This is a standard pattern for attaching metadata to functions while preserving type information.
50-50: LGTM!Exports maintain backward compatibility with the aliased re-exports (
Prompt,prompt), allowing existing consumers to continue using either naming convention.libs/sdk/src/utils/metadata.utils.ts (1)
1-2: Re-export approach is correct and properly implemented.The utilities
getMetadata,setMetadata, andhasAsyncWithare correctly re-exported from@frontmcp/diwith compatible signatures. The implementation maintains backward compatibility while following the barrel file export pattern specified in the coding guidelines. Active usage inlibs/sdk/src/provider/provider.registry.ts(via the barrel file) confirms the implementation works as intended.libs/sdk/src/common/interfaces/base.interface.ts (1)
1-16: Re-export addresses backward compatibility concern correctly.The file properly re-exports the 14 DI types from
@frontmcp/difor backward compatibility. Verification confirms all previously exported types are included—the speculation about missing "Resolve-related types" and "Mutable" was unfounded, as these were never exported as types from this module.libs/di/src/registry/container.ts (1)
93-111: LGTM!The constructor logic is correct and properly initializes the container with fallback normalizer, parent handling, and async timeout configuration.
libs/di/src/__tests__/async-with.decorator.spec.ts (1)
1-112: LGTM!Comprehensive test coverage for the AsyncWith decorator, including metadata storage, lazy evaluation for TDZ avoidance, inheritance, and readonly tuple support. The tests are well-structured and cover both typical and edge cases.
libs/di/src/__tests__/container.spec.ts (4)
19-60: LGTM!The helper function
createContainerand basic construction tests provide good foundation. Tests properly verify empty containers, provider initialization, and parent-child relationships.
62-224: LGTM!Thorough coverage of all provider kinds (CLASS_TOKEN, VALUE, FACTORY, CLASS) with appropriate dependency injection scenarios. Tests properly use reflection metadata to simulate decorated classes.
331-405: LGTM!Comprehensive testing of scoped providers (GLOBAL/CONTEXT) and AsyncWith decorator support. Tests verify both synchronous and asynchronous initialization patterns, including proper dependency injection.
510-658: LGTM!Excellent coverage of CONTEXT-scoped providers, including session isolation, dependency injection from GLOBAL scope, various provider types (factory, value, class), and pre-built provider handling. Error cases are properly tested.
libs/sdk/src/common/interfaces/prompt.interface.ts (2)
4-4: LGTM: DI types properly centralized.The import refactoring to source
FuncType,Token, andTypefrom@frontmcp/dialigns with the PR objective to centralize dependency injection primitives. This improves maintainability by eliminating duplication.
24-24: The addition ofFunctionalPromptTypeto thePromptTypeunion is backward-compatible and does not require type guard updates. Type discrimination in the codebase is based on runtime behavior checks (symbol metadata andisClass()), not union-based pattern matching. The existing code innormalizePrompt()and schema validation already handles this pattern correctly through symbol metadata checks (item[FrontMcpPromptTokens.type]), which are applied uniformly to all function-style prompts regardless of their specific type annotation.Likely an incorrect or invalid review comment.
libs/sdk/src/common/records/provider.record.ts (1)
1-10: Re-export strategy is correct and complete.The types being re-exported from
@frontmcp/dimatch the source definitions exactly and are properly made available to the SDK codebase. Since this is the initial implementation, there are no previous local definitions to compare. All imports throughout the SDK correctly resolve through the index.ts exports, maintaining backward compatibility with the centralized record types guideline.libs/sdk/src/common/metadata/provider.metadata.ts (2)
16-16: This is a justified breaking change aligning with coding guidelines.Changing from
.passthrough()to.strict()is intentional per the learning guideline to "use.strict()on all Zod schemas for validation." All existing provider metadata declarations in the codebase conform to the strict schema—they only define the fields specified in the schema (id, name, description, scope). No code currently passes unknown properties to provider metadata, so this change will not break existing code.
2-2: External types are identical, but verify the schema validation change from.passthrough()to.strict().The external
ProviderScopeenum andProviderMetadatainterface from@frontmcp/dimatch the previous local definitions exactly. However, the schema validation was changed from.passthrough()to.strict(), which rejects unknown properties instead of allowing them. The usage atannotated-class.schema.ts:43works around this by explicitly calling.passthrough()to override the strict behavior, but this strict validation inprovider.decorator.ts:10may cause issues if callers pass extra properties expecting them to be ignored.⛔ Skipped due to learnings
Learnt from: CR Repo: agentfront/frontmcp PR: 0 File: libs/uipack/CLAUDE.md:0-0 Timestamp: 2025-12-24T00:41:55.397Z Learning: Applies to libs/uipack/src/**/*.{ts,tsx} : Use `.strict()` on all Zod schemas for validationLearnt from: CR Repo: agentfront/frontmcp PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-19T02:04:46.475Z Learning: FrontMCP is a TypeScript-first schema validation framework - all types should align with MCP protocol definitionsLearnt from: CR Repo: agentfront/frontmcp PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-19T02:04:46.475Z Learning: Applies to libs/sdk/src/**/*.ts : Use `changeScope` property instead of `scope` in change events to avoid confusion with Scope class
…lated utility functions
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
libs/sdk/src/agent/__tests__/agent.registry.test.tslibs/sdk/src/agent/agent.utils.tslibs/sdk/src/agent/flows/call-agent.flow.tslibs/sdk/src/common/metadata/tool.metadata.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts
💤 Files with no reviewable changes (1)
- libs/sdk/src/tool/flows/tools-list.flow.ts
✅ Files skipped from review due to trivial changes (1)
- libs/sdk/src/agent/flows/call-agent.flow.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/sdk/src/agent/agent.utils.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use strict TypeScript mode with noanytypes without strong justification - useunknowninstead for generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific errors instead
Use type parameters with constraints instead of unconstrained generics
Do not mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.tslibs/sdk/src/common/metadata/tool.metadata.tslibs/sdk/src/agent/__tests__/agent.registry.test.ts
libs/sdk/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/src/**/*.ts: MCP response types should use strict MCP protocol types (GetPromptResult, ReadResourceResult) instead ofunknown
Use specific error classes with MCP error codes instead of generic errors
UsegetCapabilities()method for dynamic capability exposure instead of hardcoding capabilities
UsechangeScopeproperty instead ofscopein change events to avoid confusion with Scope class
Validate URIs per RFC 3986 at metadata level using proper validation
Validate hook flows and fail fast on invalid hook configurations with specific error messages
Files:
libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.tslibs/sdk/src/common/metadata/tool.metadata.tslibs/sdk/src/agent/__tests__/agent.registry.test.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.tslibs/sdk/src/common/metadata/tool.metadata.tslibs/sdk/src/agent/__tests__/agent.registry.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Achieve 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including error cases and constructor validation
Include error classinstanceofchecks in tests to validate error handling
Files:
libs/sdk/src/agent/__tests__/agent.registry.test.ts
🧠 Learnings (2)
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/src/react/hooks/**/*.{ts,tsx} : MCP bridge hooks (useMcpBridge, useCallTool, useToolInput, useToolOutput, useTheme) must be properly typed and handle loading/error states
Applied to files:
libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.tslibs/sdk/src/common/metadata/tool.metadata.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: FrontMCP is a TypeScript-first schema validation framework - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/common/metadata/tool.metadata.ts
🔇 Additional comments (3)
libs/sdk/src/agent/__tests__/agent.registry.test.ts (1)
365-365: Update outdated documentation—the tool naming already uses standard names without a prefix.The test expectation is correct. The codebase already generates tool names without the
'use-agent:'prefix (seeagentToolName()in agent.utils.ts, which explicitly states: "Agents use standard tool names without any prefix"). However, outdated comments in the source files contradict this:
- agent.registry.ts (line 321): Claims agents are exposed with name
use-agent:<agent_id>- call-agent.flow.ts (line 120): Claims agents are invoked with
use-agent:<agent_id>nameUpdate these comments to reflect the actual implementation, which uses the agent ID directly (e.g.,
'tool-agent'not'use-agent:tool-agent').Likely an incorrect or invalid review comment.
libs/sdk/src/common/metadata/tool.metadata.ts (1)
253-254: This review comment's core concerns cannot be substantiated. A codebase-wide search found:
- No
AGENT_TOOL_PREFIXconstant or import anywhere- No reserved prefix validation references or validation logic
- No evidence of removed validation
The schema at lines 253-254 (
id: z.string().optional()andname: z.string().min(1)) is simple and straightforward with no special handling or refinements. There is no indication that validation was loosened or removed—only basic type validation exists. The claim of a breaking change due to removed reserved-prefix validation cannot be verified.Likely an incorrect or invalid review comment.
libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts (1)
26-52: LGTM! Robust error handling with proper schema validation.The error handling implementation is comprehensive and correct:
- Properly distinguishes FlowControl (control flow mechanism) from actual errors
- Validates FlowControl.respond output against CallToolResultSchema to ensure MCP compliance
- Uses InternalMcpError for proper MCP error formatting
- Includes detailed logging for debugging
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
libs/sdk/src/common/interfaces/app.interface.ts (1)
4-6: Empty interface intentionally retained as marker - acknowledged.The eslint-disable comment and JSDoc documentation adequately justify the marker interface pattern. The static analysis hint can be safely ignored in this case.
🧹 Nitpick comments (1)
libs/di/src/utils/provider.utils.ts (1)
79-102: Avoidanycasts—use constrained type parameters or helpers.Multiple
anycasts are used to access.nameand.metadataproperties (lines 83, 96, 100). Per coding guidelines, avoidanywithout strong justification.🔎 Refactor to improve type safety
Define a helper type or use type guards:
type WithName = { name?: string }; type WithMetadata = { metadata?: ProviderMetadata }; // Then in the function: const metadata: ProviderMetadata = { name: getMetadata(tokens.name, item) ?? (item as WithName).name ?? 'UnknownProvider', scope: getMetadata(tokens.scope, item), description: tokens.description ? getMetadata(tokens.description, item) : undefined, id: tokens.id ? getMetadata(tokens.id, item) : undefined, }; // For plain classes: const staticMeta = (item as WithMetadata).metadata; return { kind: ProviderKind.CLASS_TOKEN, provide: item as Type, metadata: staticMeta ?? { name: (item as WithName).name ?? 'UnknownProvider' }, } satisfies ProviderClassTokenRecord;This maintains the same runtime behavior while improving compile-time type safety.
As per coding guidelines, use
unknownor constrained types instead ofany.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
libs/di/src/utils/provider.utils.tslibs/sdk/src/adapter/adapter.utils.tslibs/sdk/src/common/interfaces/app.interface.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/sdk/src/adapter/adapter.utils.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use strict TypeScript mode with noanytypes without strong justification - useunknowninstead for generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific errors instead
Use type parameters with constraints instead of unconstrained generics
Do not mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/di/src/utils/provider.utils.tslibs/sdk/src/common/interfaces/app.interface.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/di/src/utils/provider.utils.tslibs/sdk/src/common/interfaces/app.interface.ts
libs/sdk/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/src/**/*.ts: MCP response types should use strict MCP protocol types (GetPromptResult, ReadResourceResult) instead ofunknown
Use specific error classes with MCP error codes instead of generic errors
UsegetCapabilities()method for dynamic capability exposure instead of hardcoding capabilities
UsechangeScopeproperty instead ofscopein change events to avoid confusion with Scope class
Validate URIs per RFC 3986 at metadata level using proper validation
Validate hook flows and fail fast on invalid hook configurations with specific error messages
Files:
libs/sdk/src/common/interfaces/app.interface.ts
🧠 Learnings (5)
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
libs/di/src/utils/provider.utils.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: FrontMCP is a TypeScript-first schema validation framework - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/common/interfaces/app.interface.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/src/**/*.{ts,tsx} : Never import React-free utilities from frontmcp/ui; use frontmcp/uipack for bundling, build tools, platform adapters, and theme utilities
Applied to files:
libs/sdk/src/common/interfaces/app.interface.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx} : Avoid using `any` type without justification in TypeScript
Applied to files:
libs/sdk/src/common/interfaces/app.interface.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/src/universal/**/*.{ts,tsx} : Universal app shell (UniversalApp, FrontMCPProvider) must provide platform-agnostic React context and initialization
Applied to files:
libs/sdk/src/common/interfaces/app.interface.ts
🧬 Code graph analysis (2)
libs/di/src/utils/provider.utils.ts (1)
libs/sdk/src/provider/provider.utils.ts (1)
normalizeProvider(12-64)
libs/sdk/src/common/interfaces/app.interface.ts (1)
libs/sdk/src/common/metadata/app.metadata.ts (1)
AppMetadata(197-197)
🪛 Biome (2.1.2)
libs/sdk/src/common/interfaces/app.interface.ts
[error] 6-6: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
⏰ 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: Lint & Format Checks
- GitHub Check: Build Libraries
🔇 Additional comments (4)
libs/sdk/src/common/interfaces/app.interface.ts (1)
1-2: LGTM on the import migration.The import of
TypeandValueTypefrom@frontmcp/dicorrectly aligns with the new DI library surface being introduced in this PR.libs/di/src/utils/provider.utils.ts (3)
26-47: LGTM! Well-defined interfaces for provider normalization.The
ProviderTokensandProviderNormalizerOptionsinterfaces are clearly documented and properly typed. The optional fields fordescriptionandidprovide flexibility while maintaining type safety.
138-144: Error handling properly addresses circular reference concern.The try-catch block correctly handles potential circular references in
JSON.stringify, falling back toString(item). This addresses the previous review feedback.
186-204: LGTM! Invocation token resolution is correct.The function properly handles all provider kinds and returns the appropriate dependency tokens for each case. The switch statement provides exhaustive coverage.
…iders and enhance factory provider dependency filtering
# Conflicts: # libs/sdk/src/tool/flows/tools-list.flow.ts # libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.