feat(sdk): Add support for mcp prompts list and content #75
Conversation
# Conflicts: # docs/draft/docs/servers/apps.mdx
WalkthroughAdds a complete Prompts subsystem: registry with ownership/adoption/indexing and export resolution, PromptInstance and PromptContext, flows for listing/getting prompts, utilities, events, domain errors, scope and adapter integration, extensive tests and mocks, demo prompts/resources, docs updates, and CI workflow tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GetPromptFlow
participant PromptRegistry
participant PromptInstance
participant PromptContext
Client->>GetPromptFlow: requests prompts/get (name + args + auth)
GetPromptFlow->>GetPromptFlow: parseInput (validate method & payload)
GetPromptFlow->>PromptRegistry: findByName(name)
PromptRegistry-->>GetPromptFlow: PromptInstance | undefined
alt prompt found
GetPromptFlow->>PromptInstance: create(parsedArgs, { authInfo, scope, providers })
PromptInstance-->>PromptContext: instantiate context (owner bound)
GetPromptFlow->>PromptContext: execute(parsedArgs)
PromptContext-->>GetPromptFlow: raw output (or throw)
GetPromptFlow->>PromptInstance: safeParseOutput(raw)
PromptInstance-->>GetPromptFlow: ParsedPromptResult
GetPromptFlow-->>Client: respond(parsed result)
else not found
GetPromptFlow-->>Client: throw PromptNotFoundError
end
sequenceDiagram
participant Client
participant PromptsListFlow
participant PromptRegistry
participant Resolver
Client->>PromptsListFlow: requests prompts/list (cursor?)
PromptsListFlow->>PromptsListFlow: parseInput (validate method)
PromptsListFlow->>PromptRegistry: getPrompts()
PromptRegistry-->>PromptsListFlow: PromptInstance[]
PromptsListFlow->>Resolver: resolveConflicts(base names + owner keys)
Resolver-->>PromptsListFlow: resolved names (prefixed if conflicts)
PromptsListFlow->>PromptsListFlow: map to descriptors (name/title/args/icons)
PromptsListFlow-->>Client: respond(list of prompt descriptors)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas needing extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
frontegg-david
changed the base branch from
main
to
add-support-for-resources
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (24)
libs/sdk/src/__test-utils__/mocks/prompt-registry.mock.ts (2)
11-12: Consider stronger typing for the overrides parameter.Per coding guidelines, strict TypeScript mode should avoid
anytypes without justification. Consider using a more specific type or at least documenting whyanyis needed here.-export function createMockPromptRegistry(overrides: Partial<any> = {}) { +export function createMockPromptRegistry(overrides: Partial<Record<string, unknown>> = {}) {
55-61: Unused parameteroptsinexportResolvedNames.The
optsparameter is declared but never used in the implementation. Consider removing it or documenting if it's for API compatibility.- exportResolvedNames: jest.fn((opts?: any) => { + exportResolvedNames: jest.fn(() => {libs/sdk/src/index.ts (1)
15-18: Prompt hook exports look consistent with existing hook pattern
PromptHookandListPromptsHookfollow the sameFlowHooksOf('<domain>:<action>')pattern as tools/resources; this is a sensible public API surface. Make sure the high‑level SDK docs draft underdocs/draft/docs/**is updated to mention these new hooks so consumers discover them.libs/sdk/src/common/interfaces/internal/registry.interface.ts (1)
140-152: Prompt registry interface mostly matches implementation; consider exposingincludeHiddenAdding
owner: EntryOwnerRefandfindByNamebrings the interface in line with the new PromptRegistry behavior. Since the implementation also supportsgetPrompts(includeHidden?: boolean), you may want to update the interface signature togetPrompts(includeHidden?: boolean): PromptEntry[];so callers using the interface (e.g., viaScope.prompts) can opt into hidden prompts without casting.libs/sdk/src/prompt/__tests__/prompt.registry.test.ts (2)
5-33: Consider reusing shared provider-registry mocks to avoid driftThe local
createMockProviderRegistry/createMockScopehelpers duplicate behavior that likely exists under__test-utils__/mocks(e.g., provider-registry mocks). Reusing the shared mocks would reduce duplication and keep these tests aligned if provider behavior evolves.
403-418: StrengthenlistByOwnerassertion to validate behavior
expect(owned.length).toBeGreaterThanOrEqual(0)will always pass and doesn’t verify thatlistByOwneractually returns prompts for the given owner. If the intended behavior is that prompts fromownerare discoverable, assert a specific length (e.g.,toBe(1)) or at leasttoBeGreaterThan(0)and validate the prompt name/owner.libs/sdk/src/prompt/__tests__/prompt.instance.test.ts (1)
29-456: PromptInstance behavior is well covered; a couple of edge paths could be addedThe tests exercise constructor wiring, metadata exposure, argument parsing, multiple output shapes, and
safeParseOutputsuccess cases; they give good confidence in PromptInstance’s core behavior. As a follow‑up, consider adding (1) a small test forcreate()to ensure the correct context type is instantiated for eachPromptKind, and (2) asafeParseOutputfailure test that feeds clearly invalid output and asserts{ success: false, error instanceof Error }to lock in the error path.libs/sdk/src/prompt/__tests__/prompt.utils.test.ts (2)
191-194: Consider verifying the expected text for undefined input.The test verifies that a message is created but doesn't assert on the expected text content. For consistency with other primitive tests (null → 'null', number → '42', boolean → 'true'), consider adding an assertion for what text is expected for
undefined.it('should handle undefined raw value', () => { const result = buildParsedPromptResult(undefined, metadata); expect(result.messages).toHaveLength(1); + expect((result.messages[0].content as any).text).toBe('undefined'); });
209-237: Consider adding assertions for actual dependency content.The
promptDiscoveryDepstests only verify that the result is an array, but don't assert on the actual dependency values or expected behavior. This makes it difficult to catch regressions if the function's behavior changes.it('should return empty array for class without dependencies', () => { @Prompt({ name: 'no-deps', arguments: [], }) class NoDeps { execute(args: Record<string, string>) { return { messages: [] }; } } const record = normalizePrompt(NoDeps); const deps = promptDiscoveryDeps(record); - expect(Array.isArray(deps)).toBe(true); + expect(deps).toEqual([]); }); it('should return empty array for function prompt', () => { const functionPrompt = prompt({ name: 'func-deps', arguments: [], })((args) => ({ messages: [], })); const record = normalizePrompt(functionPrompt); const deps = promptDiscoveryDeps(record); - expect(Array.isArray(deps)).toBe(true); + expect(deps).toEqual([]); });libs/sdk/src/prompt/flows/prompts-list.flow.ts (1)
76-77: Consider typingparamsmore strictly.The
paramsvariable is typed asany. Since you're only accessingparams?.cursor, you could use a more specific type or inline the access.let method!: string; - let params: any; + let params: { cursor?: string } | undefined;libs/sdk/src/prompt/flows/__tests__/prompts-list.flow.test.ts (2)
369-443: Add specific error type assertions.The invalid configuration tests verify that an error is thrown but don't assert on the specific error type. Per the coding guidelines, error class
instanceofchecks should be tested. This ensures the correct error types are thrown for each scenario.+import { InvalidMethodError, InvalidInputError } from '../../../errors'; // ... in the tests: it('should throw InvalidMethodError for wrong method', async () => { const promptRegistry = createMockPromptRegistry(); const input = { request: { method: 'tools/list', params: {} }, ctx: {}, }; const result = await runFlow(input, createMockDependencies(promptRegistry)); expect(result.success).toBe(false); expect(result.error).toBeDefined(); - // Error is either InvalidMethodError with method name or InvalidInputError - expect(result.error).toBeInstanceOf(Error); + expect(result.error).toBeInstanceOf(InvalidMethodError); }); it('should throw InvalidInputError for missing request', async () => { const promptRegistry = createMockPromptRegistry(); const input = { ctx: {}, }; const result = await runFlow(input, createMockDependencies(promptRegistry)); expect(result.success).toBe(false); - expect(result.error).toBeDefined(); + expect(result.error).toBeInstanceOf(InvalidInputError); });Based on learnings, error class
instanceofchecks must be tested.
82-86: Consider testing stage isolation.The
runFlowhelper runs all stages sequentially. Consider adding tests that verify individual stage behavior in isolation, or test that errors in early stages prevent later stages from running.libs/sdk/src/prompt/prompt.events.ts (1)
26-28: Consider adding error handling for listener exceptions.If a listener throws an exception, the current implementation will stop notifying remaining listeners. Consider wrapping each listener call in a try-catch to ensure all listeners are notified regardless of individual failures.
emit(e: PromptChangeEvent) { - for (const l of [...this.listeners]) l(e); + for (const l of [...this.listeners]) { + try { + l(e); + } catch (err) { + // Optionally log error but continue notifying other listeners + console.error('PromptEmitter listener error:', err); + } + } }libs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.ts (1)
366-472: Addinstanceofchecks for error classes.The error scenario tests verify that errors occur but don't confirm the specific error class types. Per coding guidelines, error class
instanceofchecks should be tested.Consider adding assertions like:
it('should throw InvalidMethodError for wrong method', async () => { // ... expect(result.success).toBe(false); expect(result.error).toBeDefined(); - // Error is either InvalidMethodError with method name or InvalidInputError - expect(result.error).toBeInstanceOf(Error); + // Import the specific error class and verify + expect(result.error).toBeInstanceOf(InvalidMethodError); });Apply similar changes for
PromptNotFoundError,InvalidInputError, and other domain-specific error classes.libs/sdk/src/prompt/prompt.instance.ts (1)
73-81: Consider exhaustive type checking pattern.The default case correctly throws for unhandled kinds. For stronger type safety, consider the exhaustive check pattern:
case PromptKind.FUNCTION: return new FunctionPromptContext(record as PromptFunctionTokenRecord, promptCtorArgs); default: - // This should be unreachable if all PromptKind values are handled - throw new Error(`Unhandled prompt kind: ${(record as any).kind}`); + // Exhaustive check - TypeScript will error if a new PromptKind is added + const _exhaustive: never = record; + throw new Error(`Unhandled prompt kind: ${(record as { kind: string }).kind}`); }This ensures compile-time errors if new
PromptKindvalues are added without updating this switch.libs/sdk/src/common/interfaces/prompt.interface.ts (1)
52-52:_errorfield is set but never accessed.The
_errorfield is assigned infail()(line 112) but has no getter or public accessor. If this is intended for debugging or error inspection, consider adding a getter:private _error?: Error; + +public get error(): Error | undefined { + return this._error; +}Otherwise, if it's not needed, consider removing it to avoid confusion.
libs/sdk/src/prompt/prompt.utils.ts (3)
78-85: Add exhaustiveness check for switch statement.The switch on
rec.kindlacks adefaultcase. While TypeScript may infer exhaustiveness fromPromptKind, an explicit check ensures future-proofing if new kinds are added.export function promptDiscoveryDeps(rec: PromptRecord): Token[] { switch (rec.kind) { case PromptKind.FUNCTION: return depsOfFunc(rec.provide, 'discovery'); case PromptKind.CLASS_TOKEN: return depsOfClass(rec.provide, 'discovery'); + default: { + const _exhaustive: never = rec.kind; + throw new Error(`Unknown prompt kind: ${_exhaustive}`); + } } }
50-73: Consider adding a type guard or documenting theanyusage.The
item: anyparameter is justified given the need to handle multiple input types (function-prompt or class), but per coding guidelines requiring strict TypeScript mode with noanywithout justification, consider either:
- Adding a JSDoc explaining the
anyusage- Creating a union type for valid prompt inputs
+/** + * Normalize any prompt input (class or function) to a PromptRecord. + * @param item - Accepts function-prompt handlers or class decorators; `any` is used + * because the input may be a decorated function, a class, or an invalid type. + */ export function normalizePrompt(item: any): PromptRecord {
125-136: Type assertion without validation may cause runtime issues.In
normalizePromptMessage, the check at line 127 validatesmsg.roleandmsg.contentexist but doesn't validate their types. A malformed object with non-stringroleor invalidcontentstructure would pass through.function normalizePromptMessage(msg: any): PromptMessage { // If already in correct format - if (msg && msg.role && msg.content) { + if ( + msg && + typeof msg.role === 'string' && + msg.content && + typeof msg.content === 'object' + ) { return msg as PromptMessage; }libs/sdk/src/prompt/flows/get-prompt.flow.ts (2)
115-119: MutatingrawInputfor hook filtering is a side effect that may cause issues.Directly mutating
this.rawInputto inject_promptOwnerIdcouples the flow to internal hook filtering mechanics and could cause unexpected behavior ifrawInputis used elsewhere or if it's frozen/sealed.Consider using a dedicated context object or flow state for hook filtering metadata.
// Store prompt owner ID in the flow input for hook filtering. // This mutation allows hooks to filter by prompt owner during execution. if (prompt.owner) { - (this.rawInput as any)._promptOwnerId = prompt.owner.id; + // Consider storing in state instead: + this.state.set('promptOwnerId', prompt.owner.id); }If mutation is required for hook infrastructure compatibility, add a comment explaining why state cannot be used.
155-176: RedundantparseArgumentscall in execute stage.
parseArgumentsis called in bothcreatePromptContext(line 135) andexecute(line 169). If the prompt context already has parsed arguments, re-parsing is unnecessary overhead and could produce inconsistent results if state changes between stages.Consider storing parsed arguments in state during
createPromptContextand reusing them:@Stage('createPromptContext') async createPromptContext() { // ... try { // Parse and validate arguments const parsedArgs = prompt.parseArguments(input.arguments); + this.state.set('parsedArgs', parsedArgs); // ... } @Stage('execute') async execute() { // ... try { - // Parse arguments again for execution - const parsedArgs = this.state.required.prompt.parseArguments(input.arguments); + const parsedArgs = this.state.required.parsedArgs; promptContext.output = await promptContext.execute(parsedArgs);libs/sdk/src/prompt/prompt.registry.ts (3)
188-193: Use type-safe check forhideFromDiscovery.The check
(meta as any).hideFromDiscoverybypasses type safety. SincePromptMetadatamay or may not include this field, use optional chaining with a type guard or extend the type.return all .filter((r) => { const meta = r.instance.metadata; - const hidden = 'hideFromDiscovery' in meta && (meta as any).hideFromDiscovery === true; + const hidden = 'hideFromDiscovery' in meta && meta.hideFromDiscovery === true; return !hidden || includeHidden; }) .map((r) => r.instance);If
hideFromDiscoveryis a valid metadata property, ensure it's added to thePromptMetadatatype definition.
327-337: Extract magic number and nested function for clarity.The
disambiguatefunction is nested insideexportResolvedNames, which is fine for encapsulation, but the magic number10000should be a named constant.+const MAX_DISAMBIGUATE_ATTEMPTS = 10000; + // Inside exportResolvedNames: function disambiguate(candidate: string, pool: Map<string, any>, cfg: Required<PromptExportOptions>): string { if (!pool.has(candidate)) return candidate; - const maxAttempts = 10000; + const maxAttempts = MAX_DISAMBIGUATE_ATTEMPTS; let n = 2;
430-432: Empty catch block silently swallows errors.The empty
catchblock at line 430 silently ignores any errors fromgetMetadata. While this may be intentional for robustness, consider at least logging at debug/trace level for troubleshooting.- } catch { - /* ignore */ + } catch (e) { + // Metadata extraction is best-effort; log at trace level for debugging + // this.logger?.trace?.('providerIdOf: metadata extraction failed', e); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
libs/sdk/src/__test-utils__/mocks/index.ts(1 hunks)libs/sdk/src/__test-utils__/mocks/prompt-registry.mock.ts(1 hunks)libs/sdk/src/app/instances/app.local.instance.ts(1 hunks)libs/sdk/src/common/entries/prompt.entry.ts(1 hunks)libs/sdk/src/common/entries/scope.entry.ts(2 hunks)libs/sdk/src/common/interfaces/internal/registry.interface.ts(1 hunks)libs/sdk/src/common/interfaces/prompt.interface.ts(1 hunks)libs/sdk/src/errors/index.ts(1 hunks)libs/sdk/src/errors/mcp.error.ts(1 hunks)libs/sdk/src/index.ts(1 hunks)libs/sdk/src/plugin/plugin.registry.ts(1 hunks)libs/sdk/src/prompt/__tests__/prompt.decorator.test.ts(1 hunks)libs/sdk/src/prompt/__tests__/prompt.instance.test.ts(1 hunks)libs/sdk/src/prompt/__tests__/prompt.registry.test.ts(1 hunks)libs/sdk/src/prompt/__tests__/prompt.utils.test.ts(1 hunks)libs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.ts(1 hunks)libs/sdk/src/prompt/flows/__tests__/prompts-list.flow.test.ts(1 hunks)libs/sdk/src/prompt/flows/get-prompt.flow.ts(1 hunks)libs/sdk/src/prompt/flows/prompts-list.flow.ts(1 hunks)libs/sdk/src/prompt/index.ts(1 hunks)libs/sdk/src/prompt/prompt.events.ts(1 hunks)libs/sdk/src/prompt/prompt.instance.ts(1 hunks)libs/sdk/src/prompt/prompt.registry.ts(1 hunks)libs/sdk/src/prompt/prompt.types.ts(1 hunks)libs/sdk/src/prompt/prompt.utils.ts(1 hunks)libs/sdk/src/scope/scope.instance.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript mode with no
anytypes without strong justification
Files:
libs/sdk/src/index.tslibs/sdk/src/__test-utils__/mocks/index.tslibs/sdk/src/app/instances/app.local.instance.tslibs/sdk/src/common/entries/scope.entry.tslibs/sdk/src/errors/index.tslibs/sdk/src/prompt/index.tslibs/sdk/src/prompt/flows/prompts-list.flow.tslibs/sdk/src/prompt/__tests__/prompt.utils.test.tslibs/sdk/src/prompt/__tests__/prompt.decorator.test.tslibs/sdk/src/prompt/__tests__/prompt.instance.test.tslibs/sdk/src/prompt/prompt.utils.tslibs/sdk/src/prompt/flows/__tests__/prompts-list.flow.test.tslibs/sdk/src/__test-utils__/mocks/prompt-registry.mock.tslibs/sdk/src/prompt/__tests__/prompt.registry.test.tslibs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/prompt/prompt.events.tslibs/sdk/src/prompt/prompt.types.tslibs/sdk/src/common/interfaces/prompt.interface.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/plugin/plugin.registry.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/common/entries/prompt.entry.tslibs/sdk/src/common/interfaces/internal/registry.interface.tslibs/sdk/src/prompt/prompt.instance.tslibs/sdk/src/prompt/prompt.registry.ts
**/src/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/src/index.ts: Barrel exports (index.ts) must export everything users need without legacy exports or aliased names
Don't add backwards compatibility exports in new libraries; use direct imports instead
Files:
libs/sdk/src/index.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/index.tslibs/sdk/src/__test-utils__/mocks/index.tslibs/sdk/src/app/instances/app.local.instance.tslibs/sdk/src/common/entries/scope.entry.tslibs/sdk/src/errors/index.tslibs/sdk/src/prompt/index.tslibs/sdk/src/prompt/flows/prompts-list.flow.tslibs/sdk/src/prompt/__tests__/prompt.utils.test.tslibs/sdk/src/prompt/__tests__/prompt.decorator.test.tslibs/sdk/src/prompt/__tests__/prompt.instance.test.tslibs/sdk/src/prompt/prompt.utils.tslibs/sdk/src/prompt/flows/__tests__/prompts-list.flow.test.tslibs/sdk/src/__test-utils__/mocks/prompt-registry.mock.tslibs/sdk/src/prompt/__tests__/prompt.registry.test.tslibs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/prompt/prompt.events.tslibs/sdk/src/prompt/prompt.types.tslibs/sdk/src/common/interfaces/prompt.interface.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/plugin/plugin.registry.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/common/entries/prompt.entry.tslibs/sdk/src/common/interfaces/internal/registry.interface.tslibs/sdk/src/prompt/prompt.instance.tslibs/sdk/src/prompt/prompt.registry.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{test,spec}.{ts,tsx}: In Jest test files, don't use prefixes like 'PT-001' in test names; use clean, descriptive names
Constructor validation tests must not be skipped and error classinstanceofchecks must be tested
Test all code paths including error scenarios
Files:
libs/sdk/src/prompt/__tests__/prompt.utils.test.tslibs/sdk/src/prompt/__tests__/prompt.decorator.test.tslibs/sdk/src/prompt/__tests__/prompt.instance.test.tslibs/sdk/src/prompt/flows/__tests__/prompts-list.flow.test.tslibs/sdk/src/prompt/__tests__/prompt.registry.test.tslibs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.ts
🧠 Learnings (5)
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/src/index.ts : Don't add backwards compatibility exports in new libraries; use direct imports instead
Applied to files:
libs/sdk/src/__test-utils__/mocks/index.ts
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/src/index.ts : Barrel exports (index.ts) must export everything users need without legacy exports or aliased names
Applied to files:
libs/sdk/src/__test-utils__/mocks/index.tslibs/sdk/src/errors/index.tslibs/sdk/src/prompt/index.ts
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Test all code paths including error scenarios
Applied to files:
libs/sdk/src/prompt/__tests__/prompt.utils.test.tslibs/sdk/src/prompt/flows/__tests__/prompts-list.flow.test.tslibs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.ts
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Constructor validation tests must not be skipped and error class `instanceof` checks must be tested
Applied to files:
libs/sdk/src/prompt/__tests__/prompt.instance.test.ts
📚 Learning: 2025-11-25T19:34:36.034Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:34:36.034Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : In Jest test files, don't use prefixes like 'PT-001' in test names; use clean, descriptive names
Applied to files:
libs/sdk/src/prompt/__tests__/prompt.instance.test.tslibs/sdk/src/prompt/__tests__/prompt.registry.test.ts
🧬 Code graph analysis (11)
libs/sdk/src/app/instances/app.local.instance.ts (1)
libs/sdk/src/prompt/prompt.registry.ts (1)
PromptRegistry(33-440)
libs/sdk/src/common/entries/scope.entry.ts (1)
libs/sdk/src/common/interfaces/internal/registry.interface.ts (1)
PromptRegistryInterface(140-152)
libs/sdk/src/prompt/flows/prompts-list.flow.ts (1)
libs/sdk/src/errors/index.ts (2)
InvalidInputError(12-12)InvalidMethodError(14-14)
libs/sdk/src/prompt/__tests__/prompt.instance.test.ts (3)
libs/sdk/src/__test-utils__/mocks/provider-registry.mock.ts (1)
createMockProviderRegistry(12-51)libs/sdk/src/prompt/prompt.utils.ts (1)
normalizePrompt(50-73)libs/sdk/src/prompt/prompt.instance.ts (1)
PromptInstance(22-133)
libs/sdk/src/prompt/flows/__tests__/prompts-list.flow.test.ts (1)
libs/sdk/src/__test-utils__/mocks/prompt-registry.mock.ts (3)
createMockPromptRegistry(11-77)createMockPromptEntry(82-162)addPromptToMock(167-185)
libs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.ts (1)
libs/sdk/src/__test-utils__/mocks/prompt-registry.mock.ts (3)
createMockPromptRegistry(11-77)createMockPromptEntry(82-162)addPromptToMock(167-185)
libs/sdk/src/prompt/prompt.types.ts (2)
libs/sdk/src/prompt/prompt.instance.ts (1)
PromptInstance(22-133)libs/sdk/src/prompt/prompt.registry.ts (1)
PromptRegistry(33-440)
libs/sdk/src/common/interfaces/prompt.interface.ts (3)
libs/sdk/src/common/interfaces/internal/registry.interface.ts (1)
ProviderRegistryInterface(82-93)libs/sdk/src/app/instances/app.local.instance.ts (1)
providers(53-55)libs/sdk/src/scope/scope.instance.ts (1)
providers(149-151)
libs/sdk/src/scope/scope.instance.ts (1)
libs/sdk/src/prompt/prompt.registry.ts (1)
PromptRegistry(33-440)
libs/sdk/src/common/entries/prompt.entry.ts (1)
libs/sdk/src/common/interfaces/prompt.interface.ts (1)
PromptInterface(13-15)
libs/sdk/src/prompt/prompt.instance.ts (5)
libs/sdk/src/scope/scope.instance.ts (3)
Scope(30-192)providers(149-151)hooks(141-143)libs/sdk/src/hooks/hook.registry.ts (1)
HookRegistry(19-193)libs/sdk/src/common/entries/prompt.entry.ts (2)
PromptGetExtra(21-23)ParsedPromptResult(25-25)libs/sdk/src/common/interfaces/prompt.interface.ts (2)
scope(77-79)PromptCtorArgs(26-32)libs/sdk/src/prompt/prompt.utils.ts (1)
buildParsedPromptResult(90-120)
🔇 Additional comments (34)
libs/sdk/src/errors/index.ts (1)
10-11: LGTM!The new prompt error exports follow the established pattern and maintain consistency with other error types like
ToolNotFoundErrorandResourceNotFoundError. Based on learnings, the barrel exports are clean without legacy exports or aliases.libs/sdk/src/errors/mcp.error.ts (1)
173-202: LGTM!The new error classes follow the established patterns perfectly:
PromptNotFoundErrormirrorsToolNotFoundErrorandResourceNotFoundErrorwith consistent message formatting and 404 status.PromptExecutionErrormirrorsResourceReadErrorandToolExecutionError, correctly storingoriginalErrorand overridinggetInternalMessage()to include the stack trace.libs/sdk/src/__test-utils__/mocks/prompt-registry.mock.ts (1)
1-162: Well-structured mock utilities for PromptRegistry testing.The mock factory provides comprehensive test utilities with proper Jest mocks for all registry operations. The
addPromptToMockhelper correctly rewires the mock implementations to reflect registry state changes.libs/sdk/src/__test-utils__/mocks/index.ts (1)
11-11: LGTM!The new mock export follows the established pattern and properly exposes the prompt registry mock utilities for test consumers. Based on learnings, this maintains clean barrel exports without legacy aliases.
libs/sdk/src/common/entries/scope.entry.ts (2)
18-19: LGTM!The
PromptRegistryInterfaceimport is correctly added alongside other registry interface imports.
49-50: LGTM!The abstract
promptsgetter follows the established pattern used bytoolsandresourcesgetters, maintaining API consistency across theScopeEntryclass.libs/sdk/src/plugin/plugin.registry.ts (1)
89-107: Owner propagation toPromptRegistryaligns with tools/resourcesPassing
pluginOwnerintoPromptRegistrykeeps prompt ownership consistent with tools/resources and ensures provenance/lineage features work for plugin prompts as well. No issues spotted with the initialization ordering or readiness handling.libs/sdk/src/app/instances/app.local.instance.ts (1)
34-51: App-level prompt registry wiring is consistent and ordered correctlyConstructing
this.appPromptswithappOwnerand includingthis.appPrompts.readyin thePromise.allkeeps prompts aligned with existing app tools/resources semantics and ensures everything is ready before the app is considered initialized.libs/sdk/src/prompt/__tests__/prompt.registry.test.ts (1)
35-468: Comprehensive PromptRegistry coverageThis suite exercises registration, lookup (
getPrompts,findByName,findAllByName), indexing, export naming, subscriptions, ownership, and exported lookups; it’s a solid behavioral spec for the new registry.libs/sdk/src/prompt/index.ts (1)
1-9: Prompt barrel cleanly exposes the prompt subsystemThe barrel exports the registry, instance, supporting types/events/utils, and both prompt flows without legacy aliases, so consumers can import everything prompt‑related from a single place.
libs/sdk/src/scope/scope.instance.ts (1)
27-43: Perfect. I have all the information needed to verify the review comment. Let me analyze the findings:Verification Summary:
PromptRegistry Integration Pattern: The code at lines 27-43, 95-97, and 165-167 correctly implements the pattern:
- Single
PromptRegistryinstance perScope(line 42:private scopePrompts)- Initialized once in
initialize()method (line 95)- Follows identical pattern to
ToolRegistry(line 40/89-90) andResourceRegistry(line 41/92-93)Flow Registration Mechanism:
PromptRegistry.initialize()callsscope.registryFlows(GetPromptFlow, PromptsListFlow)exactly once per PromptRegistry instance, and since each Scope has only one PromptRegistry, flows are registered once per scope.Deduplication Concern: The review's concern about "multiple PromptRegistry instances registering the same prompt flows" is not applicable:
- Multiple PromptRegistry instances exist only across different Scopes (expected and intended)
- Within a single Scope, there is exactly one PromptRegistry instance
- Each Scope has its own
FlowRegistry, so registering the same flow class across different scopes is fine- FlowRegistry uses token-based deduplication (Set/Map) within each registry instance
Idempotency: Even if
registryFlows()were called multiple times for the same token within a FlowRegistry, the Map/Set operations would be idempotent. However, this scenario doesn't occur in practice.Conclusion: The implementation is correct and the architectural concern raised is moot. The code safely follows the established pattern without any risk of unintended duplicate flow registrations.
Scope-level prompts registry follows the established pattern correctly
Importing
PromptRegistry, storing it as the singleprivate scopePromptsfield (line 42), initializing it once during scope setup (line 95), and exposing it via thepromptsgetter mirrors theToolRegistryandResourceRegistryimplementations. Each registry instance registers its flows exactly once during initialization, and each Scope maintains its own separate registries, so no duplicate flow registration occurs. The implementation is sound.libs/sdk/src/prompt/__tests__/prompt.decorator.test.ts (1)
1-349: LGTM! Comprehensive test coverage for the prompt decorator subsystem.The test suite provides excellent coverage for:
- Class-based and function-based prompt decorators
- Alias behavior (FrontMcpPrompt/frontMcpPrompt)
- Metadata storage and retrieval
- Validation (empty name rejection)
- Argument definitions (full, minimal, multiple)
- Edge cases (empty arguments, undefined optional fields)
Test names are clean and descriptive, following the coding guidelines.
libs/sdk/src/prompt/__tests__/prompt.utils.test.ts (1)
1-295: Good test coverage for prompt utilities.The test suite covers the key utility functions with various input types and edge cases. The error scenarios for
normalizePromptare tested (lines 94-98), and thebuildParsedPromptResultfunction is tested with multiple input formats.libs/sdk/src/prompt/flows/prompts-list.flow.ts (2)
1-189: Well-structured flow implementation.The four-stage flow design is clean with good separation of concerns:
parseInput: Input validation and method checkingfindPrompts: Collecting prompts from registryresolveConflicts: Name collision resolution with owner prefixingparsePrompts: Response formattingThe conflict resolution logic (lines 132-152) correctly handles name collisions by prefixing with owner ID. Logging is comprehensive throughout.
108-110: The review comment is incorrect. Theownerproperty onPromptEntryis explicitly declared as required at line 29 oflibs/sdk/src/common/entries/prompt.entry.ts:export abstract class PromptEntry extends BaseEntry<AnyPromptRecord, PromptInterface, PromptMetadata> { owner: EntryOwnerRef;Since
ownerlacks the optional marker (?), TypeScript's type system enforces that allPromptEntryinstances—including those returned fromgetPrompts()—haveownerdefined. The access toprompt.owner.idat lines 108-110 is therefore safe and requires no null check.libs/sdk/src/prompt/flows/__tests__/prompts-list.flow.test.ts (1)
1-589: Comprehensive test coverage for PromptsListFlow.The test suite provides excellent coverage including:
- Empty and populated prompt registries
- Name conflict resolution with owner prefixing
- Invalid inputs (wrong method, missing request, null input)
- Edge cases (special characters, large number of prompts, undefined fields)
- Context propagation
Test structure is clean with well-organized describe blocks.
libs/sdk/src/prompt/prompt.events.ts (1)
1-29: Clean event emitter implementation.The
PromptEmitterclass provides a simple pub/sub mechanism for prompt change events. The design is appropriate:
- Returns unsubscribe function from
on()for easy cleanup- Creates array copy before iteration to allow safe modification during emit
- Types are well-defined for change events
libs/sdk/src/prompt/prompt.types.ts (1)
1-46: Well-structured type definitions with sensible defaults.The types are cleanly defined and the
DEFAULT_PROMPT_EXPORT_OPTSprovides reasonable defaults. TheIndexedPrompttype correctly captures the provenance and ownership model.One consideration: the
source: PromptRegistryreference creates a coupling between this types file and the registry implementation. If this causes circular import issues in the future, consider using an interface abstraction instead.libs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.ts (3)
10-101: Well-structured test setup with comprehensive mocks.The
createMockDependencieshelper andrunFlowhelper provide clean abstractions for testing. The FlowControl handling in the catch block correctly distinguishes between successful responses and actual failures.
103-364: Good coverage of valid configuration scenarios.The tests thoroughly cover:
- Basic prompt execution with arguments
- Prompts with no arguments and optional arguments
- Auth info propagation
- String and array output formats
Test names are clean and descriptive, following coding guidelines.
587-774: Thorough edge case coverage.Good testing of:
- Special characters in prompt names
- Namespaced names with owner prefix
- Empty arguments
- Null and undefined outputs with expected failure behavior
The comment on line 759 documenting expected behavior is helpful.
libs/sdk/src/prompt/prompt.instance.ts (3)
22-37: Clean constructor initialization with proper wiring.The constructor properly initializes all required dependencies and sets up the instance with owner context. The async initialization pattern with
this.ready = this.initialize()follows the established pattern in the codebase.
89-113: Argument parsing logic is correct.The implementation properly:
- Validates required arguments (checking both
undefinedandnull)- Copies defined arguments to result
- Includes additional arguments not in the definition (pass-through behavior)
This allows flexibility for prompts that accept arbitrary additional parameters.
138-145: Simple and effective FunctionPromptContext wrapper.The
FunctionPromptContextcleanly wraps function-based prompts by delegating execution to the underlying function, passing both args and the context itself.libs/sdk/src/common/entries/prompt.entry.ts (2)
11-19:anytypes justified for generic record union.The
anytypes inAnyPromptRecordare acceptable here because:
Type<any>is needed to match class constructors with varied instance types- The function signature
(...args: any[]) => anyprovides flexibility for different prompt implementationsThis pattern is common for generic unions handling heterogeneous types.
21-26: Well-defined helper types for prompt operations.The
PromptGetExtra,ParsedPromptResult, andPromptSafeTransformResulttypes provide clear contracts for prompt context handling and safe parsing patterns.libs/sdk/src/common/interfaces/prompt.interface.ts (3)
34-69: Well-designed PromptContext with clear separation of concerns.The constructor properly initializes:
- Unique
runIdper execution instance- Stable
promptIdbased on metadata name- Child logger with prompt-specific context
- All required dependencies from
PromptCtorArgsThe separation of
runId(instance) vspromptId(type) is a good design for tracing and debugging.
91-108: Output management with automatic history tracking.The setter automatically records history entries with timestamps and current stage - useful for debugging and audit trails. The
respond()method cleanly records output before triggering the control flow response.
81-89: DefensivetryGetwith proper error logging.Good pattern for optional provider resolution with warning logging on failure. The error message extraction handles both Error instances and other thrown values.
libs/sdk/src/prompt/flows/get-prompt.flow.ts (2)
161-164: Silent early return whenpromptContextis missing may mask errors.In the
executestage, returning early without throwing whenpromptContextis undefined could lead to silent failures. IfcreatePromptContextsucceeded,promptContextshould always exist. If it failed, the flow should have already thrown.Is the early return intentional for a specific scenario (e.g., optional execution)? If
promptContextis required for execution, consider throwing instead:if (!promptContext) { - this.logger.warn('execute: promptContext not found, skipping execution'); - return; + this.logger.error('execute: promptContext not found'); + throw new PromptExecutionError(input.name, new Error('Prompt context not created')); }
22-36: Well-documentedz.any()usage for complex external types.The comments on lines 27-33 clearly justify the use of
z.any()for complex external types likeAuthInfo,PromptEntry, andPromptContext. This aligns with the coding guideline requiring justification foranytypes.libs/sdk/src/prompt/prompt.registry.ts (3)
63-71: Constructor initialization pattern looks correct.The pattern of storing the promise from
initialize()inthis.ready(inherited fromRegistryAbstract) is a valid lazy-init pattern. Consumers canawait registry.readywhen needed. Thefalseparameter tosuper()correctly disables auto-init so subclass fields initialize first.
168-174: Subscription callback may fire during initial adoption.The
subscribecall withimmediate: falsesets up a live listener for child changes. However, ifchildemits during the same tick (e.g., during its own initialization), this could cause unexpected reindexing. The subsequentthis.reindex()andthis.bump('reset')on lines 176-177 should handle the initial state, but verify the timing is correct.
113-113: The type cast is correct and necessary for type safety.The
instancesmap inRegistryAbstractis typed asMap<Token<Interface>, Interface>. SincePromptRegistryextendsRegistryAbstract<PromptInstance, PromptRecord, PromptType[]>, the instances map becomesMap<Token<PromptInstance>, PromptInstance>. Thetokenvariable is typed as bareToken(fromthis.tokens: Set<Token>), so the cast toToken<PromptInstance>is necessary and correct to match the map's key type parameter.
… prompt documentation
…nd mock implementations
# Conflicts: # libs/sdk/src/__test-utils__/mocks/index.ts # libs/sdk/src/app/instances/app.local.instance.ts # libs/sdk/src/auth/auth.registry.ts # libs/sdk/src/common/entries/resource.entry.ts # libs/sdk/src/common/entries/scope.entry.ts # libs/sdk/src/common/interfaces/resource.interface.ts # libs/sdk/src/errors/index.ts # libs/sdk/src/errors/mcp.error.ts # libs/sdk/src/index.ts # libs/sdk/src/plugin/plugin.registry.ts # libs/sdk/src/resource/__tests__/resource.utils.test.ts # libs/sdk/src/resource/flows/read-resource.flow.ts # libs/sdk/src/resource/flows/resource-templates-list.flow.ts # libs/sdk/src/resource/flows/resources-list.flow.ts # libs/sdk/src/resource/resource.events.ts # libs/sdk/src/resource/resource.instance.ts # libs/sdk/src/resource/resource.registry.ts # libs/sdk/src/resource/resource.types.ts # libs/sdk/src/resource/resource.utils.ts # libs/sdk/src/scope/scope.instance.ts # libs/sdk/src/transport/adapters/transport.local.adapter.ts
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…with capability notifications
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
libs/sdk/src/common/entries/prompt.entry.ts (1)
55-64: Previous type mismatch issue has been addressed.The
parseOutputsignature now correctly usesraw: unknowninstead ofGetPromptResult, matching the implementation inprompt.instance.tsand the docstring describing raw output normalization.
🧹 Nitpick comments (16)
.github/workflows/update-draft-docs.yml (1)
164-171: Align prompt guidance with verified MCP server availability.The updated prompt documentation references
mintlify_docsMCP server as available and usable (lines 164–166, 170). Once the MCP endpoint validity (flagged above) is confirmed, consider:
- Explicitly documenting how Codex should query the server (e.g., available tools, expected response format).
- Adding error-handling guidance if the MCP server is unavailable or times out.
- Providing fallback instructions for Codex if MCP queries fail.
docs/draft/docs/servers/prompts.mdx (2)
399-411: Consider showing explicit error handling patterns.The error handling example (lines 399-411) shows error messaging within the return value, which works but is implicit. Per the PR objectives mentioning
PromptNotFoundErrorandPromptExecutionError, consider whether examples should showcase throwing/catching these error classes for clarity on proper error handling in prompts.This could be clarified by adding a brief note or additional example showing proper error handling with error classes, but the current approach is functional.
443-456: Add error handling to MCP Protocol Integration section.The MCP Protocol Integration section documents the happy path for
prompts/listandprompts/getflows, but doesn't describe error cases (e.g., when a prompt is not found, validation fails, or execution fails). Given that the PR introducesPromptNotFoundErrorandPromptExecutionError, consider documenting these error scenarios in the protocol flow.When a client requests `prompts/get` with a name and arguments: 1. The SDK locates the prompt by name 2. (NEW) If prompt not found → PromptNotFoundError is returned 3. Arguments are validated against the prompt's definition 4. (NEW) If validation fails → error response with details 5. The `execute()` method is called with the arguments 6. (NEW) If execution fails → PromptExecutionError is returned 7. The return value is converted to MCP `GetPromptResult` formatThis would align better with the error classes mentioned in the PR summary.
libs/sdk/src/errors/mcp.error.ts (1)
361-389: Align prompt error behavior with existing execution/not-found errorsThe new prompt errors look consistent overall, but two small inconsistencies with existing errors are worth tightening up:
PromptNotFoundErrorcurrently uses the defaultstatusCode = 400. GivenToolNotFoundErrorandResourceNotFoundErrorboth use404, consider also passing404here for consistency and clearer semantics.PromptExecutionErrorstoresoriginalErrorbut does not overridegetInternalMessage(), unlikeResourceReadError,ToolExecutionError, andGenericServerError. This means internal logs won’t automatically include the wrapped stack trace.You can mirror the existing pattern:
export class PromptNotFoundError extends PublicMcpError { constructor(promptName: string) { - super(`Prompt not found: ${promptName}`, 'PROMPT_NOT_FOUND'); + super(`Prompt not found: ${promptName}`, 'PROMPT_NOT_FOUND', 404); } } export class PromptExecutionError extends InternalMcpError { readonly promptName: string; readonly originalError?: Error; constructor(promptName: string, cause?: Error) { super( cause ? `Prompt execution failed: ${cause.message}` : `Prompt execution failed: ${promptName}`, 'PROMPT_EXECUTION_FAILED', ); this.promptName = promptName; this.originalError = cause; } + override getInternalMessage(): string { + if (this.originalError?.stack) { + return `${this.message}\n\nOriginal error:\n${this.originalError.stack}`; + } + return this.message; + } }libs/sdk/src/common/interfaces/prompt.interface.ts (1)
34-124: PromptContext design and flow control wiring look solidThe
PromptContextabstraction cleanly encapsulates providers, scope, metadata, auth, and output/error state, and therespond/failhelpers correctly delegate toFlowControlwhile recording output and error. The automaticoutputHistorytracking on the setter is a nice touch for observability.If you want to go further later, you could:
- Return a shallow copy in
outputHistory(e.g.return [...this._outputHistory];) or freeze the array for stronger immutability guarantees.- Consider eventually differentiating
promptIdvspromptNameif you introduce a separate stable identifier.But functionally this looks good as-is.
libs/sdk/src/prompt/__tests__/prompt.instance.test.ts (1)
1-523: PromptInstance test coverage is thorough and well-aligned with implementationThis suite does a good job exercising all major
PromptInstancebehaviors: constructor wiring, metadata exposure, argument parsing (required/optional/extra/undefined), multiple output shapes,safeParseOutputsuccess/failure, and both CLASS_TOKEN and FUNCTION contexts viacreate(). It also validates record/metadata passthrough and error wrapping.If you want to tighten it further later, you could add an assertion in the
createtests thatcontext.authInfomatches the injectedmockCtx.authInfo, but overall these tests are in great shape.libs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.ts (1)
450-472: Tighten error-path expectations with explicit instanceof checksThe flow tests do a nice job covering happy paths, argument variants, auth propagation, and many edge cases. For a few of the error scenarios you currently only assert that an error is “defined” or that the run failed:
prompt execution throwing error(Lines 476–506)invalid output format(Lines 508–545)prompt returning undefined output/ rawOutput missing in finalize (Lines 760–775)Given
GetPromptFlowis explicitly mapping to domain errors likePromptExecutionErrorandInvalidOutputError, it would be safer to assert the concrete error type as well, similar to what you’re already doing forInvalidInputErrorandPromptNotFoundError. For example:expect(result.success).toBe(false); expect(result.error).toBeInstanceOf(PromptExecutionError); // or InvalidOutputError, as appropriateThis will catch regressions where those errors accidentally degrade back to generic
ErrororGenericServerError.Also applies to: 476-506, 508-545, 760-775
libs/sdk/src/prompt/prompt.instance.ts (2)
139-147: Useunknowninstead ofanyfor the execute return type.Per coding guidelines, avoid
anytypes without strong justification. Theexecutemethod should useunknownor match the interface contract.- execute(args: Record<string, string>): Promise<any> { + execute(args: Record<string, string>): Promise<unknown> { return this.record.provide(args, this); }
94-103: Consider edge case: extra arguments not in definition could mask typos.The implementation accepts additional arguments not defined in
argDefs(lines 106-112). This is flexible but could silently pass typo'd argument names. Consider logging a warning for unexpected arguments.libs/sdk/src/prompt/prompt.utils.ts (1)
101-108: Preferunknownoveranyfor therawparameter.Per coding guidelines, use
unknownfor generic type defaults. The function already handles type narrowing internally.-export function buildParsedPromptResult(raw: any, metadata: PromptMetadata): GetPromptResult { +export function buildParsedPromptResult(raw: unknown, metadata: PromptMetadata): GetPromptResult { // If already in GetPromptResult format if (raw && typeof raw === 'object' && 'messages' in raw && Array.isArray(raw.messages)) { return { - description: raw.description ?? metadata.description, + description: (raw as { description?: string }).description ?? metadata.description, messages: raw.messages as PromptMessage[], }; }libs/sdk/src/prompt/flows/get-prompt.flow.ts (1)
15-38: Appropriate use ofz.any()with explanatory comments.The comments on lines 27-35 justify the
anyusage for complex external types (AuthInfo,PromptEntry,PromptContext). For line 17 (ctx: z.any()), consider adding a similar comment explaining the context type.const inputSchema = z.object({ request: GetPromptRequestSchema, + // z.any() used because ctx is a complex RequestHandlerExtra type from @modelcontextprotocol/sdk ctx: z.any(), });libs/sdk/src/common/entries/prompt.entry.ts (1)
28-39: Consider using definite assignment assertion or making properties abstract.The
owner,name, andfullNameproperties are declared but not initialized in the abstract class. Since subclasses must set these in their constructors, consider using the definite assignment assertion (!) or documenting this contract.export abstract class PromptEntry extends BaseEntry<AnyPromptRecord, PromptInterface, PromptMetadata> { - owner: EntryOwnerRef; + owner!: EntryOwnerRef; /** * The name of the prompt, as declared in the metadata. */ - name: string; + name!: string; /** * The full name of the prompt, including the owner name as prefix. */ - fullName: string; + fullName!: string;Note: Per coding guidelines, non-null assertions should be avoided, but for abstract class properties that subclasses must initialize, this is an acceptable pattern. Alternatively, make these
abstractgetters.libs/sdk/src/prompt/prompt.registry.ts (4)
156-181: Adoption logic from child registries looks solid; watch out for potential cyclesThe
adoptFromChildimplementation correctly:
- Guards against double‑adoption with
this.children.has(child).- Clones
IndexedPromptrows viarelineagewhile preserving the originalinstancereference andsource.- Subscribes to child changes and recomputes adopted rows + indexes on updates.
Two small considerations:
- If other parts of the system allow a child to also adopt this registry (or form longer cycles), you could end up with cyclical subscription/update chains. If that’s a possibility, you may want an additional lineage/owner‑based guard against cycles.
_childOwneris currently unused and intentionally named as such; if you don’t expect to need it for prefixing, consider removing it to reduce noise.Overall, the adoption & live‑update behavior itself looks good.
188-205: MakehideFromDiscoveryhandling more robust to unexpected metadata shapes
getPromptsdoes:const meta = r.instance.metadata; const hidden = 'hideFromDiscovery' in meta && (meta as { hideFromDiscovery?: boolean }).hideFromDiscovery === true;If
metadatawere evernullor a primitive at runtime,'hideFromDiscovery' in metawould throw. Given this is library code, it’s safer to defensively gate:const meta = r.instance.metadata as { hideFromDiscovery?: boolean } | null | undefined; const hidden = !!meta && meta.hideFromDiscovery === true;This maintains the behavior while avoiding hard failures on unexpected metadata.
269-345: Align export‑name errors with SDK error types and considermaxLen/suffix edge‑case
exportResolvedNamesanddisambiguateare nicely structured and cover most collision scenarios, but there are two points to tighten:
- Generic
Errorusethrow new Error( `Failed to disambiguate name "${candidate}" after ${MAX_DISAMBIGUATE_ATTEMPTS} attempts`, );Per the SDK guidelines, this should use a specific error class extending
PublicMcpError(or your internal registry error type) with an appropriate MCP error code. That keeps error handling consistent across the SDK and easier to surface to clients.
maxLenand suffix truncationWhen
candidateis already atcfg.maxLen, appending${sepFor(cfg.case)}${n}and then applyingensureMaxLencan truncate the suffix back off, yielding the same string for multiplenvalues. In that degenerate case you’ll exhaustMAX_DISAMBIGUATE_ATTEMPTSand throw even though there might be viable shorter names.If that’s a concern for your workloads, consider:
- Reserving a few characters for the numeric suffix when computing the base; or
- Falling back to a hashed suffix when repeated truncation keeps colliding.
For most realistic name lengths this won’t bite, but it’s a low‑level edge worth being explicit about.
355-374: Event payload naming vs guidelines; good use ofgetCapabilities()The subscription pipeline is clean:
subscribesupportsimmediatesnapshots and an optional filter.bumpcentralizes versioning and event emission.getCapabilities()correctly deriveslistChangedfromhasAny(), so capabilities reflect actual registry state rather than being hardcoded in adapters (this matches the stored learnings about usinggetCapabilities()for dynamic capability exposure). Based on learnings, this aligns with the registry capability guideline.One naming concern:
- Events are emitted with
{ kind, scope: 'global', ... }and the subscriber also assumes ascopefield.- The SDK guidelines call for using
changeScopeinstead ofscopein change event types to avoid confusion with theScopeclass.If
PromptChangeEventhas already been updated to usechangeScope, this code will fail type‑checking and should be switched tochangeScope. If it still usesscope, consider scheduling a follow‑up pass to rename the field in the event type and here for consistency with the rest of the SDK.Also applies to: 445-458
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
.github/workflows/update-draft-docs.yml(3 hunks)docs/draft/docs/guides/enclave.mdx(1 hunks)docs/draft/docs/plugins/codecall-plugin.mdx(1 hunks)docs/draft/docs/servers/prompts.mdx(1 hunks)libs/sdk/src/__test-utils__/mocks/prompt-registry.mock.ts(1 hunks)libs/sdk/src/__test-utils__/mocks/provider-registry.mock.ts(3 hunks)libs/sdk/src/common/entries/prompt.entry.ts(1 hunks)libs/sdk/src/common/interfaces/internal/registry.interface.ts(1 hunks)libs/sdk/src/common/interfaces/prompt.interface.ts(1 hunks)libs/sdk/src/errors/index.ts(1 hunks)libs/sdk/src/errors/mcp.error.ts(1 hunks)libs/sdk/src/prompt/__tests__/prompt.decorator.test.ts(1 hunks)libs/sdk/src/prompt/__tests__/prompt.instance.test.ts(1 hunks)libs/sdk/src/prompt/__tests__/prompt.registry.test.ts(1 hunks)libs/sdk/src/prompt/__tests__/prompt.utils.test.ts(1 hunks)libs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.ts(1 hunks)libs/sdk/src/prompt/flows/__tests__/prompts-list.flow.test.ts(1 hunks)libs/sdk/src/prompt/flows/get-prompt.flow.ts(1 hunks)libs/sdk/src/prompt/flows/prompts-list.flow.ts(1 hunks)libs/sdk/src/prompt/prompt.events.ts(1 hunks)libs/sdk/src/prompt/prompt.instance.ts(1 hunks)libs/sdk/src/prompt/prompt.registry.ts(1 hunks)libs/sdk/src/prompt/prompt.utils.ts(1 hunks)libs/sdk/src/scope/scope.instance.ts(4 hunks)libs/sdk/src/transport/adapters/transport.local.adapter.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- docs/draft/docs/plugins/codecall-plugin.mdx
- docs/draft/docs/guides/enclave.mdx
🚧 Files skipped from review as they are similar to previous changes (7)
- libs/sdk/src/prompt/tests/prompt.decorator.test.ts
- libs/sdk/src/prompt/flows/tests/prompts-list.flow.test.ts
- libs/sdk/src/prompt/tests/prompt.registry.test.ts
- libs/sdk/src/test-utils/mocks/prompt-registry.mock.ts
- libs/sdk/src/prompt/flows/prompts-list.flow.ts
- libs/sdk/src/prompt/prompt.events.ts
- libs/sdk/src/prompt/tests/prompt.utils.test.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with noanytypes without strong justification; preferunknownfor generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific error classes instead
Files:
libs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/common/interfaces/internal/registry.interface.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/errors/index.tslibs/sdk/src/prompt/prompt.instance.tslibs/sdk/src/common/interfaces/prompt.interface.tslibs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.tslibs/sdk/src/prompt/prompt.utils.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/prompt/__tests__/prompt.instance.test.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/common/entries/prompt.entry.tslibs/sdk/src/__test-utils__/mocks/provider-registry.mock.ts
libs/sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/**/*.{ts,tsx}: Create shared base classes for common functionality (e.g., ExecutionContextBase) to reduce code duplication
Use specific error classes extending PublicMcpError with MCP error codes instead of generic errors
UsegetCapabilities()for dynamic capability exposure in registries instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopein change event types to avoid confusion with the Scope class
Validate URIs per RFC 3986 at metadata level using Zod schema validation
Fail fast on invalid hook flows by validating hooks match their entry type during initialization
Use type parameters with constraints instead of unconstrained oranydefaults in generic types
Do not mutate rawInput in flows - use state.set() for flow state management instead
Files:
libs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/common/interfaces/internal/registry.interface.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/errors/index.tslibs/sdk/src/prompt/prompt.instance.tslibs/sdk/src/common/interfaces/prompt.interface.tslibs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.tslibs/sdk/src/prompt/prompt.utils.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/prompt/__tests__/prompt.instance.test.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/common/entries/prompt.entry.tslibs/sdk/src/__test-utils__/mocks/provider-registry.mock.ts
libs/sdk/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Centralize record types in common/records directory and import from there rather than module-specific files
Files:
libs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/common/interfaces/internal/registry.interface.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/errors/index.tslibs/sdk/src/prompt/prompt.instance.tslibs/sdk/src/common/interfaces/prompt.interface.tslibs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.tslibs/sdk/src/prompt/prompt.utils.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/prompt/__tests__/prompt.instance.test.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/common/entries/prompt.entry.tslibs/sdk/src/__test-utils__/mocks/provider-registry.mock.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/adapters/transport.local.adapter.tslibs/sdk/src/common/interfaces/internal/registry.interface.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/errors/index.tslibs/sdk/src/prompt/prompt.instance.tslibs/sdk/src/common/interfaces/prompt.interface.tslibs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.tslibs/sdk/src/prompt/prompt.utils.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/prompt/__tests__/prompt.instance.test.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/common/entries/prompt.entry.tslibs/sdk/src/__test-utils__/mocks/provider-registry.mock.ts
docs/draft/docs/**
⚙️ CodeRabbit configuration file
docs/draft/docs/**: This folder holds the draft/source docs that humans are expected to edit. When authors want to add or change documentation, they should do it here. The Codex workflow uses these drafts, together with the code diff, to generate the latest docs under docs/docs/. As a reviewer: - Encourage contributors to add/update content here instead of docs/docs/. - It is fine to do structural/content feedback here (clarity, examples, etc).
Files:
docs/draft/docs/servers/prompts.mdx
docs/**
⚙️ CodeRabbit configuration file
docs/**: Repository documentation for the SDK, using MDX and hosted by Mintlify. See more specific rules for: - docs/docs/** (latest rendered docs, automation-only) - docs/v/** (archived versions, read-only) - docs/draft/docs/** (human-editable drafts) - docs/blogs/** (blogs, human edited) - docs/docs.json (Mintlify navigation)
Files:
docs/draft/docs/servers/prompts.mdx
**/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Export barrel exports in index.ts with clean, direct exports; do not add legacy or backwards compatibility exports
Files:
libs/sdk/src/errors/index.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Achieve 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test constructor validation, error class instanceof checks, and all code paths including error conditions
Files:
libs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.tslibs/sdk/src/prompt/__tests__/prompt.instance.test.ts
🧠 Learnings (8)
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Use `getCapabilities()` for dynamic capability exposure in registries instead of hardcoding capabilities in adapters
Applied to files:
libs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/__test-utils__/mocks/provider-registry.mock.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Use `changeScope` instead of `scope` in change event types to avoid confusion with the Scope class
Applied to files:
libs/sdk/src/scope/scope.instance.ts
📚 Learning: 2025-11-16T22:47:05.139Z
Learnt from: frontegg-david
Repo: agentfront/frontmcp PR: 50
File: docs/docs/v/0.1/getting-started/welcome.mdx:2-3
Timestamp: 2025-11-16T22:47:05.139Z
Learning: In the agentfront/frontmcp repository, the docs/docs/v/** directory governance rule prohibiting manual edits applies to already-frozen releases, but does not preclude initial setup and bootstrap commits that establish the versioned docs structure.
Applied to files:
.github/workflows/update-draft-docs.yml
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Use specific error classes extending PublicMcpError with MCP error codes instead of generic errors
Applied to files:
libs/sdk/src/errors/index.tslibs/sdk/src/errors/mcp.error.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Fail fast on invalid hook flows by validating hooks match their entry type during initialization
Applied to files:
libs/sdk/src/errors/index.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/src/**/*.{ts,tsx} : Centralize record types in common/records directory and import from there rather than module-specific files
Applied to files:
libs/sdk/src/prompt/prompt.utils.tslibs/sdk/src/common/entries/prompt.entry.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Create shared base classes for common functionality (e.g., ExecutionContextBase) to reduce code duplication
Applied to files:
libs/sdk/src/prompt/prompt.utils.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to **/*.test.{ts,tsx} : Test constructor validation, error class instanceof checks, and all code paths including error conditions
Applied to files:
libs/sdk/src/prompt/__tests__/prompt.instance.test.ts
🧬 Code graph analysis (6)
libs/sdk/src/common/interfaces/prompt.interface.ts (3)
libs/sdk/src/common/interfaces/internal/registry.interface.ts (1)
ProviderRegistryInterface(82-93)libs/sdk/src/app/instances/app.local.instance.ts (1)
providers(53-55)libs/sdk/src/scope/scope.instance.ts (1)
providers(149-151)
libs/sdk/src/prompt/flows/__tests__/get-prompt.flow.test.ts (2)
libs/sdk/src/__test-utils__/mocks/prompt-registry.mock.ts (3)
createMockPromptRegistry(11-77)createMockPromptEntry(82-162)addPromptToMock(167-185)libs/sdk/src/errors/mcp.error.ts (3)
InvalidInputError(223-240)PromptNotFoundError(368-372)PromptExecutionError(377-389)
libs/sdk/src/prompt/prompt.utils.ts (3)
libs/sdk/src/common/interfaces/prompt.interface.ts (1)
PromptType(17-17)libs/sdk/src/prompt/prompt.instance.ts (1)
getMetadata(49-51)libs/sdk/src/utils/index.ts (3)
isClass(34-34)depsOfFunc(34-34)depsOfClass(34-34)
libs/sdk/src/prompt/__tests__/prompt.instance.test.ts (3)
libs/sdk/src/prompt/prompt.utils.ts (1)
normalizePrompt(56-79)libs/sdk/src/prompt/prompt.instance.ts (1)
PromptInstance(22-134)libs/sdk/src/__test-utils__/mocks/provider-registry.mock.ts (2)
createMockProviderRegistry(71-118)createMockOwner(60-66)
libs/sdk/src/common/entries/prompt.entry.ts (1)
libs/sdk/src/common/interfaces/prompt.interface.ts (1)
PromptInterface(13-15)
libs/sdk/src/__test-utils__/mocks/provider-registry.mock.ts (1)
libs/sdk/src/__test-utils__/fixtures/scope.fixtures.ts (1)
createMockScope(14-41)
🔇 Additional comments (18)
.github/workflows/update-draft-docs.yml (2)
62-92: Verify the MCP server endpoint and configuration validity.The new MCP configuration setup introduces several details that need validation before deployment:
- Line 82: The endpoint
https://mintlify.com/docs/mcplooks like a documentation URL rather than an operational MCP server. Confirm this is the correct MCP server endpoint.- Line 84: The custom header
X-MCP-Readonlyis not a standard MCP protocol header. Verify this is supported by the Mintlify MCP server implementation.- Line 77: Enabling
rmcp_client = truefor HTTP/OAuth support—confirm this feature flag is necessary and available in your Codex/MCP setup.- Timeouts (lines 85–86): The startup timeout of 30s and tool timeout of 60s—confirm these are appropriate for the Mintlify server's response characteristics.
Additionally, has this MCP server configuration been tested end-to-end with Codex to ensure it works as expected in the workflow?
64-90: Good: RUNNER_TEMP usage and output threading.The use of
${{ runner.temp }}to create an isolated, temporary Codex home directory is a solid approach for sandboxing MCP configuration per workflow run. The output (codex_home) is correctly threaded into the Codex action step (line 366), ensuring consistency.docs/draft/docs/servers/prompts.mdx (2)
200-262: I'll help you verify the documented context properties and service resolution API for prompts. Let me search the codebase to confirm these features are accurately documented.
<function_calls>
#!/bin/bashSearch for prompt context class/interface definitions
rg -n "interface.*PromptContext|class.*PromptContext|type PromptContext" --type ts --type tsx
Search for prompt base class with context methods
rg -n "class.*Prompt\b" --type ts --type tsx -A 10 | head -50
</function_calls><function_calls>
#!/bin/bashFind prompt-related files
fd -t f -e ts -e tsx "prompt" | grep -v node_modules | head -20
</function_calls><function_calls>
#!/bin/bashList repository structure to find source files
ls -la
Find TypeScript/source files
find . -name ".ts" -o -name ".tsx" | grep -v node_modules | grep -v dist | head -30
</function_calls>
118-168: Verification confirms all documented return value conversions are accurate and implemented correctly.The review comment requests verification that the four documented return formats are actually supported. After examining the SDK codebase:
Findings:
All four return formats documented in lines 118-168 are correctly implemented:
- Simple string (line 124 conversion claim): Verified in
buildParsedPromptResult(prompt.utils.ts:111-115) — strings are wrapped in a user message- Message array (line 127-136): Verified (prompt.utils.ts:119-123) — each message is normalized via
normalizePromptMessage- Full MCP format (line 138-157): Verified (prompt.utils.ts:103-108) — already-formatted results are preserved
- Object return (line 167 conversion claim): Verified (prompt.utils.ts:127-130) — objects are JSON-serialized to text in a user message
The conversion pipeline is:
execute()→executeOutput()→prompt.safeParseOutput()→buildParsedPromptResult().No issues found with the documentation accuracy.
libs/sdk/src/transport/adapters/transport.local.adapter.ts (1)
52-59: Prompts capability wiring matches existing patternUsing
this.scope.prompts.getCapabilities()alongsideresources.getCapabilities()is consistent with the existing design and the guideline to avoid hardcoded capabilities in adapters. No issues here.libs/sdk/src/scope/scope.instance.ts (1)
28-29: PromptRegistry integration into Scope follows existing registry patternsThe new
scopePromptsregistry is wired consistently with tools/resources: constructed withscopeProvidersandscopeRef, awaited ininitialize(), and exposed via a dedicatedpromptsgetter. Assuming existing callers already respectawait scope.readybefore accessing registries (which they must for tools/resources), this addition should be safe.If there are any code paths that construct a
Scopeand immediately create aLocalTransportAdapter(or otherwise accessscope.prompts) without awaitingscope.ready, it’s worth double‑checking those call sites for potential race conditions.Also applies to: 42-43, 95-97, 165-167
libs/sdk/src/errors/index.ts (1)
18-21: Public error surface expanded – ensure docs/API references are updatedRe‑exporting
DependencyNotFoundError,InvalidHookFlowError,PromptNotFoundError, andPromptExecutionErrorfrom the main errors barrel is a good move for usability. Since this grows the public SDK API, please ensure the corresponding docs underdocs/draft/docs/**are updated to describe these errors and when they’re emitted.libs/sdk/src/common/interfaces/internal/registry.interface.ts (1)
140-152: PromptRegistryInterface extension is consistent with other registriesThe
PromptRegistryInterfaceupdates (owner field,getPrompts(includeHidden?: boolean), andfindByName) line up with the patterns already used for tools and resources and match how the mocks and flows consume the registry.Just make sure all concrete
PromptRegistryimplementations and any custom registries in plugins are updated to implementowner, respectincludeHidden, and providefindByNameto avoid subtle runtime type mismatches.libs/sdk/src/prompt/prompt.instance.ts (1)
27-37: LGTM on constructor and initialization.The constructor properly initializes owner, providers, and computes
fullName. The asyncinitialize()pattern correctly handles hook registration with proper flow filtering.libs/sdk/src/prompt/prompt.utils.ts (2)
56-79: Theanytype usage is appropriately documented.The JSDoc comment on lines 50-54 explains why
anyis intentional here—to handle both decorator patterns and provide meaningful error messages. The implementation correctly narrows types before use.
84-96: Well-implemented exhaustive check pattern.The exhaustive switch with
nevertype assertion ensures compile-time safety if newPromptKindvalues are added.libs/sdk/src/__test-utils__/mocks/provider-registry.mock.ts (2)
71-118: Good improvements to type safety in the mock registry.The changes correctly use
Map<Token, unknown>instead ofany, and the mock properly returnsundefinedfor missing tokens rather than throwing. The addition ofgetActiveScope,getRegistries, andaddRegistryaligns with the registry interface updates.
26-55: I'll verify the duplication claim by locating and comparing thecreateMockScopefunctions in both files.
<function_calls>
#!/bin/bashLocate scope.fixtures.ts
fd -type f "scope.fixtures.ts" --type ts
</function_calls>Let me search for the file more broadly:
<function_calls>
#!/bin/bashSearch for any file with "scope" and "fixtures" in the name
fd -type f "fixtures" --type ts | grep -i scope
</function_calls>Let me try a different approach:
<function_calls>
#!/bin/bashSearch for createMockScope definitions
rg -n "export function createMockScope" --type ts
</function_calls>libs/sdk/src/prompt/flows/get-prompt.flow.ts (1)
196-226: Solid finalize stage with proper error handling.The stage correctly validates state, uses
safeParseOutputfor safe parsing, and throws domain-specificInvalidOutputErroron failure. The structured logging captures useful debug context.libs/sdk/src/common/entries/prompt.entry.ts (1)
11-19: Well-typed union for both prompt kinds.
AnyPromptRecordappropriately usesType<unknown>and(...args: unknown[]) => unknownto cover both class-based and function-based prompts without resorting toany.libs/sdk/src/prompt/prompt.registry.ts (3)
27-31: Verify../regsitryimport path forRegistryAbstractThe segment
from '../regsitry'looks like it might be a typo for../registry. If the directory/module isn’t actually spelledregsitry, this will break compilation for all registries. Please confirm the path and align it with the real module name to avoid subtle import failures.
223-262: Indexing and lookup maps are straightforward and appropriateThe
listAllIndexed/listAllInstances/listByOwnerhelpers plusreindex()give you:
- A single source of truth (
IndexedPrompt) for both local and adopted prompts.- O(1) lookups by qualified id, name, owner, and owner+name.
- Deterministic iteration order based on
listAllIndexed().The logic in
reindex()is clean and side‑effect free beyond the four maps, which should make future maintenance low‑risk.
378-419: Helper utilities (makeRow,relineage,providerIdOf,dedupLineage) look correct
makeRowandrelineagecorrectly deriveownerKey,qualifiedName, andqualifiedIdfrom lineage + token, while keepinginstancereferences stable.relineage’s use ofdedupLineageto remove only adjacent duplicates preserves meaningful repeated owner segments while avoiding double‑prefixing from adoption.providerIdOfis safely wrapped in atry/catchand treats provider IDs as best‑effort metadata, which is appropriate for name prefixing.hasAnyleverages bothlistAllIndexed()andtokens.sizeso capabilities can be accurate even before adoption completes.These helpers are cohesive and align well with the registry’s responsibilities.
Also applies to: 422-447, 463-471
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
libs/sdk/src/prompt/flows/get-prompt.flow.ts (1)
118-122: Do not mutaterawInputin flows.Per coding guidelines, rawInput should not be mutated. Use
state.set()for flow state management instead. This issue was flagged in a previous review.Apply this diff to use state instead of mutating rawInput:
// Store prompt owner ID in the flow input for hook filtering. - // This mutation allows hooks to filter by prompt owner during execution. - if (prompt.owner) { - (this.rawInput as any)._promptOwnerId = prompt.owner.id; - } + // Store prompt owner ID in state for hook filtering during execution. + this.state.set('promptOwnerId', prompt.owner?.id);You'll also need to add
promptOwnerIdto the state schema:const stateSchema = z.object({ input: z.object({ name: z.string().min(1), arguments: z.record(z.string()).optional(), }), + promptOwnerId: z.string().optional(), // z.any() used because AuthInfo is a complex external type from @modelcontextprotocol/sdk authInfo: z.any().optional() as z.ZodType<AuthInfo>,docs/draft/docs/servers/prompts.mdx (1)
74-95: Verify the functional prompt builder API before publishing.This section documents a functional builder pattern. As previously flagged, the documented usage should be verified against the actual SDK implementation in
libs/sdk/src/common/decorators/prompt.decorator.tsto ensure the type signatures and runtime behavior align.
🧹 Nitpick comments (3)
libs/sdk/src/prompt/prompt.instance.ts (1)
145-149: Return type cast may not match all function prompt implementations.The cast to
Promise<GetPromptResult>assumes all function prompts return this type directly. However,buildParsedPromptResultinprompt.utils.tshandles various return types (string, array, object). This cast could mask type mismatches at compile time.Consider using
unknownor the actual return type and lettingparseOutputhandle normalization:- execute(args: Record<string, string>): Promise<GetPromptResult> { - return this.record.provide(args, this) as Promise<GetPromptResult>; + execute(args: Record<string, string>): Promise<unknown> { + return this.record.provide(args, this); }This aligns with the
parseOutputstage that already normalizes various return types.libs/sdk/src/prompt/prompt.utils.ts (1)
137-155: Message normalization assumes valid structure after type guards.The function checks for
roleandcontentproperties with string/object types, then casts toPromptMessage. This trusts that the structure matches the MCP schema without deeper validation.Consider validating the
rolevalue against allowed roles ('user'|'assistant') for stricter type safety:if ( msg && typeof msg === 'object' && 'role' in msg && 'content' in msg && typeof (msg as Record<string, unknown>)['role'] === 'string' && typeof (msg as Record<string, unknown>)['content'] === 'object' ) { + const role = (msg as Record<string, unknown>)['role']; + if (role !== 'user' && role !== 'assistant') { + // Fall through to default user message + } else { return msg as PromptMessage; + } }libs/sdk/src/common/entries/prompt.entry.ts (1)
28-39: Consider using abstract getters or constructor initialization instead of definite assignment assertions.Lines 29, 34, and 39 use definite assignment assertions (
!). Per coding guidelines, non-null assertions should be avoided. For abstract class members that must be set by subclasses, consider:
- Abstract getters (preferred for enforced implementation):
- owner!: EntryOwnerRef; - name!: string; - fullName!: string; + abstract get owner(): EntryOwnerRef; + abstract get name(): string; + abstract get fullName(): string;
- Constructor-based initialization in the concrete subclass with proper validation.
This pattern ensures compile-time enforcement that subclasses provide these values.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.md(1 hunks)docs/draft/docs/servers/prompts.mdx(1 hunks)docs/draft/docs/servers/resources.mdx(2 hunks)docs/draft/docs/servers/tools.mdx(2 hunks)libs/sdk/src/common/entries/prompt.entry.ts(1 hunks)libs/sdk/src/errors/mcp.error.ts(1 hunks)libs/sdk/src/prompt/flows/get-prompt.flow.ts(1 hunks)libs/sdk/src/prompt/prompt.instance.ts(1 hunks)libs/sdk/src/prompt/prompt.utils.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/draft/docs/servers/resources.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/sdk/src/errors/mcp.error.ts
🧰 Additional context used
📓 Path-based instructions (6)
docs/draft/docs/**
⚙️ CodeRabbit configuration file
docs/draft/docs/**: This folder holds the draft/source docs that humans are expected to edit. When authors want to add or change documentation, they should do it here. The Codex workflow uses these drafts, together with the code diff, to generate the latest docs under docs/docs/. As a reviewer: - Encourage contributors to add/update content here instead of docs/docs/. - It is fine to do structural/content feedback here (clarity, examples, etc).
Files:
docs/draft/docs/servers/tools.mdxdocs/draft/docs/servers/prompts.mdx
docs/**
⚙️ CodeRabbit configuration file
docs/**: Repository documentation for the SDK, using MDX and hosted by Mintlify. See more specific rules for: - docs/docs/** (latest rendered docs, automation-only) - docs/v/** (archived versions, read-only) - docs/draft/docs/** (human-editable drafts) - docs/blogs/** (blogs, human edited) - docs/docs.json (Mintlify navigation)
Files:
docs/draft/docs/servers/tools.mdxdocs/draft/docs/servers/prompts.mdx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with noanytypes without strong justification; preferunknownfor generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific error classes instead
Files:
libs/sdk/src/prompt/prompt.utils.tslibs/sdk/src/prompt/prompt.instance.tslibs/sdk/src/common/entries/prompt.entry.tslibs/sdk/src/prompt/flows/get-prompt.flow.ts
libs/sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/**/*.{ts,tsx}: Create shared base classes for common functionality (e.g., ExecutionContextBase) to reduce code duplication
Use specific error classes extending PublicMcpError with MCP error codes instead of generic errors
UsegetCapabilities()for dynamic capability exposure in registries instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopein change event types to avoid confusion with the Scope class
Validate URIs per RFC 3986 at metadata level using Zod schema validation
Fail fast on invalid hook flows by validating hooks match their entry type during initialization
Use type parameters with constraints instead of unconstrained oranydefaults in generic types
Do not mutate rawInput in flows - use state.set() for flow state management instead
Files:
libs/sdk/src/prompt/prompt.utils.tslibs/sdk/src/prompt/prompt.instance.tslibs/sdk/src/common/entries/prompt.entry.tslibs/sdk/src/prompt/flows/get-prompt.flow.ts
libs/sdk/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Centralize record types in common/records directory and import from there rather than module-specific files
Files:
libs/sdk/src/prompt/prompt.utils.tslibs/sdk/src/prompt/prompt.instance.tslibs/sdk/src/common/entries/prompt.entry.tslibs/sdk/src/prompt/flows/get-prompt.flow.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/prompt/prompt.utils.tslibs/sdk/src/prompt/prompt.instance.tslibs/sdk/src/common/entries/prompt.entry.tslibs/sdk/src/prompt/flows/get-prompt.flow.ts
🧠 Learnings (3)
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/src/**/*.{ts,tsx} : Centralize record types in common/records directory and import from there rather than module-specific files
Applied to files:
libs/sdk/src/prompt/prompt.utils.tslibs/sdk/src/common/entries/prompt.entry.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Create shared base classes for common functionality (e.g., ExecutionContextBase) to reduce code duplication
Applied to files:
libs/sdk/src/prompt/prompt.utils.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Do not mutate rawInput in flows - use state.set() for flow state management instead
Applied to files:
libs/sdk/src/prompt/flows/get-prompt.flow.ts
🧬 Code graph analysis (3)
libs/sdk/src/prompt/prompt.utils.ts (3)
libs/sdk/src/common/interfaces/prompt.interface.ts (1)
PromptType(17-17)libs/sdk/src/prompt/prompt.instance.ts (1)
getMetadata(50-52)libs/sdk/src/utils/index.ts (3)
isClass(34-34)depsOfFunc(34-34)depsOfClass(34-34)
libs/sdk/src/common/entries/prompt.entry.ts (1)
libs/sdk/src/common/interfaces/prompt.interface.ts (1)
PromptInterface(13-15)
libs/sdk/src/prompt/flows/get-prompt.flow.ts (1)
libs/sdk/src/errors/mcp.error.ts (5)
InvalidInputError(223-240)InvalidMethodError(269-273)PromptNotFoundError(368-372)PromptExecutionError(377-396)InvalidOutputError(245-264)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (22)
docs/draft/docs/servers/tools.mdx (2)
7-11: LGTM - Clear introduction with protocol reference.The updated opening paragraph with the MCP Tools specification reference provides helpful context for developers.
97-140: MCP specification link date verified as current.The date 2025-11-25 in the specification link is correct—it matches the latest MCP spec release published on November 25, 2025, which includes the updated Tools features. The documentation is using the intended version.
libs/sdk/src/prompt/prompt.instance.ts (4)
23-38: Well-structured PromptInstance initialization.The constructor properly wires up providers, scope, hooks, and owner context. The deferred initialization pattern via
this.ready = this.initialize()is appropriate for async hook registration.
40-48: Appropriate hook filtering for prompt flows.The initialization correctly filters hooks to only those relevant to prompt flows (
prompts:get-promptandprompts:list-prompts).
74-84: Good exhaustive switch with runtime safety.The switch statement handles both
CLASS_TOKENandFUNCTIONkinds with a proper exhaustive check in the default case. The error message includes the actual kind value for debugging.
91-116: Argument parsing includes undefined arguments not in schema.The method validates required arguments and includes any additional arguments not defined in the metadata. This open-world approach allows flexibility but may pass through unexpected data.
Verify this is the intended behavior - allowing arbitrary extra arguments could have security implications if downstream code doesn't expect them. Consider whether extra arguments should be logged or filtered based on use case.
libs/sdk/src/prompt/prompt.utils.ts (4)
17-31: Good centralization of shared utilities via re-exports.Re-exporting naming and lineage utilities from a central location promotes code reuse and aligns with the coding guidelines for centralizing common functionality.
56-79: Theanytype usage is justified with clear documentation.The JSDoc explains why
anyis used here - to handle both decorator patterns and provide meaningful error messages. The implementation properly type-guards and throws informative errors for invalid inputs.
84-96: Exhaustive switch with compile-time and runtime safety.The exhaustive check using
nevertype ensures allPromptKindvalues are handled, with a clear runtime error for unexpected cases.
101-132: Flexible output normalization handles multiple return types.The function properly handles
GetPromptResult, string, array, and fallback cases. This provides good developer ergonomics for prompt implementations.libs/sdk/src/prompt/flows/get-prompt.flow.ts (5)
15-39: Well-defined schemas with appropriate type annotations.The schema definitions use
z.any()with proper type casts for complex external types (AuthInfo, PromptEntry, PromptContext). The comments explain whyanyis used in each case.
72-101: Robust input parsing with proper error handling.The parseInput stage validates input using Zod schema, checks the method, and stores parsed data in state. Error handling wraps Zod errors appropriately.
130-157: Good context creation with cached argument parsing.The stage parses arguments once and caches them in state (
parsedArgs) to avoid redundant parsing in the execute stage. Error handling properly wraps exceptions inPromptExecutionError.
159-179: Proper use of cached parsed arguments in execute stage.The execute stage correctly retrieves
parsedArgsfrom state, avoiding duplicate parsing. The null check forpromptContextprovides a safe early return.
197-227: Robust finalization with safe output parsing.The finalize stage properly validates state, uses
safeParseOutputfor error-safe output conversion, and throws appropriate errors with context. Therespond()call returns the MCP-compliant result.README.md (1)
451-453: Documentation links verified — all URLs are accessible.The authentication documentation links have been updated to reflect a restructured docs hierarchy. All three URLs (overview, remote, and local) are accessible and return HTTP 200, confirming the migration to the new authentication path structure is complete.
[10]: https://docs.agentfront.dev/docs/authentication/overview 'Authentication - FrontMCP' [11]: https://docs.agentfront.dev/docs/authentication/remote 'Remote OAuth - FrontMCP' [12]: https://docs.agentfront.dev/docs/authentication/local 'Local OAuth - FrontMCP'libs/sdk/src/common/entries/prompt.entry.ts (2)
11-26: LGTM on the type definitions.The types are well-designed:
AnyPromptRecordproperly usesType<unknown>and(...args: unknown[]) => unknownavoidinganyPromptSafeTransformResult<T>is a proper discriminated union for safe error handling- The
parseOutput(raw: unknown)signature correctly addresses the concern from previous reviews
41-64: Well-structured abstract method contracts.The abstract methods establish a clear contract:
createfor context instantiation with proper typing ofPromptGetExtraparseArgumentsfor validation with optional input- Both
parseOutputandsafeParseOutputvariants for flexibility in error handlingThis design allows callers to choose between exception-based and result-based error handling patterns.
docs/draft/docs/servers/prompts.mdx (4)
270-443: Excellent real-world examples demonstrating key patterns.The examples effectively showcase:
- Optional argument handling with sensible defaults
- Multi-turn conversation patterns (Interview Prep)
- Provider-based data fetching with proper error handling (Project Summary)
- Consistent use of
Record<string, string>for argument typingThese provide clear guidance for developers implementing various prompt patterns.
447-488: MCP Protocol Integration section is accurate and well-documented.The section correctly explains:
- The
prompts/listandprompts/getflows aligning with SDK implementations- The
listChangedcapability and its purpose- Automatic change notification behavior
This provides users with essential protocol context.
492-506: Best practices section provides clear and actionable guidance.The Do/Don't format effectively communicates:
- Separation of concerns between prompts (message generation) and tools (side effects)
- Importance of descriptive metadata and argument definitions
- Keeping prompts focused and avoiding complexity
Good alignment with MCP design principles.
213-236: Documentation accurately reflects the PromptContext API implementation.Verification confirms all documented properties and methods exist and are properly exposed in the
PromptContextclass:
args(line 45) ✓metadata(line 41) ✓authInfo(line 36) ✓get()method (lines 73–75) ✓tryGet()method (lines 81–89) ✓scopeproperty (lines 77–79) ✓respond()method (lines 104–108) ✓No corrections needed.
…expense management
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
libs/sdk/src/tool/tool.registry.ts (1)
391-404: Consider aligninggetCapabilitiesshape and semantics with registry guidelinesImplementation is fine, but right now
getCapabilitiesexposes onlylistChanged, and it’s coupled to whether any tools are currently registered. Given the emitter-based subscription already exists, consider:
- Returning a richer shape (e.g.,
{ subscribe: true, listChanged: this.hasAny() }) to match the documented pattern for registries.- Deciding if
listChangedshould reflect capability (can emit change events) rather than current population (has tools).This keeps
ToolRegistryconsistent with other registries and the guidance in CLAUDE.md.apps/demo/src/apps/expenses/prompts/expense-report.prompt.ts (1)
1-52: Prompt implementation looks good; consider explicit MCP return typingThe prompt metadata and
executeimplementation look correct and align with the MCP prompt result shape. To keep the demo aligned with the “MCP Response Types” guideline, consider explicitly typing the method as returning the concrete MCP result type (e.g.,Promise<GetPromptResult>) rather than relying on inference.libs/sdk/src/__test-utils__/mocks/prompt-registry.mock.ts (1)
11-77: Mocks are solid; consider addinggetCapabilitiesfor parity with real registriesThe mock registry and prompt entries cover the main methods used in flows and tests (lookups, exports, owner filtering). To keep them in sync with the real
PromptRegistry/registry guidelines, consider adding a simplegetCapabilities()implementation, for example:return { prompts, ready: Promise.resolve(), owner: { kind: 'app', id: 'test-app', ref: {} }, + getCapabilities: jest.fn(() => ({ + // mirror the pattern used in other registries + listChanged: prompts.size > 0, + })),This will make the mock safer to reuse in tests that rely on dynamic capability discovery.
Also applies to: 169-194
libs/sdk/src/prompt/flows/get-prompt.flow.ts (1)
1-227: Flow design and error handling look solid; minor typing refinements possibleThis flow matches the documented pattern well: input validation, prompt lookup, context creation, cached argument parsing, raw output capture, and
safeParseOutput-based finalization with domain-specific errors. Nice job also moving owner information into state instead of mutatingrawInput.Two small typing refinements you might consider:
In
parseInput, avoidanylocals and the definite-assignmentmethod!by destructuring directly from the parsed input:const { request, ctx } = inputSchema.parse(this.rawInput); const { method, params } = request;This removes
method!: string,params: any, andctx: anyentirely.Since
stateSchemaalready captures the full state shape, you can keep future changes safer by leaning on those inferred types (e.g., preferz.infer<typeof inputSchema>/z.infer<typeof stateSchema>helpers for local annotations if you need them later).These are purely type-safety/clarity nits; the runtime behavior looks correct.
apps/demo/src/apps/expenses/resources/expense-categories.resource.ts (1)
11-79: Hoist staticcategoriesto module scope to avoid per-call re-allocationSince
categoriesis immutable demo data, consider defining it once at the module level and just referencing it inside the handler. This avoids re-creating the array on every request and makes the static nature of the data clearer, without changing behavior.apps/demo/src/apps/expenses/resources/expense-by-id.resource.ts (1)
18-38: Be aware of non-deterministic fields in the mock expense
amountand the various timestamp fields useMath.random()/new Date(), so the sameexpenseIdwill yield different responses over time. That’s fine for a demo, but if you later add tests or documentation examples that assert on this resource’s output, you may want to switch to deterministic values or a seeded random source.docs/draft/docs/servers/tools.mdx (3)
136-148: Complete the “Complex Types” example with a tool definitionThe “Complex Types” section shows an
@Tool({ ... })decorator but omits what it decorates (class or function-style tool). For consistency with earlier sections and to avoid reader confusion, consider expanding this to:@Tool({ name: 'create-order', inputSchema: { /* ... */ }, }) class CreateOrderTool { /* execute(...) { ... } */ }or a corresponding
tool(...)functional example.
243-260: ClarifyinputSchematype to match the earlier object-shaped examplesHere
inputSchemais documented asZodSchema, but most examples above use an object of fields (e.g.,{ userId: z.string(), ... }). If the implementation accepts both a Zod schema and an object map, it may help to state that explicitly (e.g., “ZodSchemaor an object mapping field names to Zod schemas”) to keep the examples and the signature in sync.
307-337: Fix minor issues in the Tool Context exampleTwo small tweaks would improve this example:
this.fetch(url);usesurlwithout defining it; consider inlining a literal URL or declaring aconst url = ...above.- If
this.respond(value)is intended to terminate execution, it may be worth adding a short comment noting that it ends the tool call, since the subsequentreturncould otherwise look contradictory.These changes make the snippet easier to follow without altering the conceptual intent.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
CLAUDE.md(1 hunks)apps/demo/src/apps/expenses/index.ts(2 hunks)apps/demo/src/apps/expenses/prompts/categorize-expense.prompt.ts(1 hunks)apps/demo/src/apps/expenses/prompts/expense-report.prompt.ts(1 hunks)apps/demo/src/apps/expenses/prompts/index.ts(1 hunks)apps/demo/src/apps/expenses/resources/expense-by-id.resource.ts(1 hunks)apps/demo/src/apps/expenses/resources/expense-categories.resource.ts(1 hunks)apps/demo/src/apps/expenses/resources/expense-policy.resource.ts(1 hunks)apps/demo/src/apps/expenses/resources/index.ts(1 hunks)docs/draft/docs/servers/tools.mdx(2 hunks)libs/sdk/src/__test-utils__/mocks/prompt-registry.mock.ts(1 hunks)libs/sdk/src/common/entries/prompt.entry.ts(1 hunks)libs/sdk/src/common/schemas/annotated-class.schema.ts(2 hunks)libs/sdk/src/prompt/flows/get-prompt.flow.ts(1 hunks)libs/sdk/src/prompt/prompt.instance.ts(1 hunks)libs/sdk/src/tool/tool.registry.ts(1 hunks)libs/sdk/src/transport/adapters/transport.local.adapter.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/sdk/src/transport/adapters/transport.local.adapter.ts
- libs/sdk/src/common/entries/prompt.entry.ts
- libs/sdk/src/prompt/prompt.instance.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with noanytypes without strong justification; preferunknownfor generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific error classes instead
Files:
apps/demo/src/apps/expenses/index.tsapps/demo/src/apps/expenses/resources/expense-categories.resource.tslibs/sdk/src/tool/tool.registry.tsapps/demo/src/apps/expenses/resources/index.tsapps/demo/src/apps/expenses/prompts/index.tsapps/demo/src/apps/expenses/prompts/expense-report.prompt.tsapps/demo/src/apps/expenses/resources/expense-by-id.resource.tslibs/sdk/src/prompt/flows/get-prompt.flow.tsapps/demo/src/apps/expenses/resources/expense-policy.resource.tslibs/sdk/src/__test-utils__/mocks/prompt-registry.mock.tsapps/demo/src/apps/expenses/prompts/categorize-expense.prompt.tslibs/sdk/src/common/schemas/annotated-class.schema.ts
**/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Export barrel exports in index.ts with clean, direct exports; do not add legacy or backwards compatibility exports
Files:
apps/demo/src/apps/expenses/index.tsapps/demo/src/apps/expenses/resources/index.tsapps/demo/src/apps/expenses/prompts/index.ts
apps/demo/**
⚙️ CodeRabbit configuration file
apps/demo/**: apps/demo directory contains a demo application for testing purposes. It can be used as a reference for SDK usage examples.
Files:
apps/demo/src/apps/expenses/index.tsapps/demo/src/apps/expenses/resources/expense-categories.resource.tsapps/demo/src/apps/expenses/resources/index.tsapps/demo/src/apps/expenses/prompts/index.tsapps/demo/src/apps/expenses/prompts/expense-report.prompt.tsapps/demo/src/apps/expenses/resources/expense-by-id.resource.tsapps/demo/src/apps/expenses/resources/expense-policy.resource.tsapps/demo/src/apps/expenses/prompts/categorize-expense.prompt.ts
libs/sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/**/*.{ts,tsx}: Create shared base classes for common functionality (e.g., ExecutionContextBase) to reduce code duplication
Use specific error classes extending PublicMcpError with MCP error codes instead of generic errors
UsegetCapabilities()for dynamic capability exposure in registries instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopein change event types to avoid confusion with the Scope class
Validate URIs per RFC 3986 at metadata level using Zod schema validation
Fail fast on invalid hook flows by validating hooks match their entry type during initialization
Use type parameters with constraints instead of unconstrained oranydefaults in generic types
Do not mutate rawInput in flows - use state.set() for flow state management instead
Files:
libs/sdk/src/tool/tool.registry.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/__test-utils__/mocks/prompt-registry.mock.tslibs/sdk/src/common/schemas/annotated-class.schema.ts
libs/sdk/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Centralize record types in common/records directory and import from there rather than module-specific files
Files:
libs/sdk/src/tool/tool.registry.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/__test-utils__/mocks/prompt-registry.mock.tslibs/sdk/src/common/schemas/annotated-class.schema.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/tool/tool.registry.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/__test-utils__/mocks/prompt-registry.mock.tslibs/sdk/src/common/schemas/annotated-class.schema.ts
docs/draft/docs/**
⚙️ CodeRabbit configuration file
docs/draft/docs/**: This folder holds the draft/source docs that humans are expected to edit. When authors want to add or change documentation, they should do it here. The Codex workflow uses these drafts, together with the code diff, to generate the latest docs under docs/docs/. As a reviewer: - Encourage contributors to add/update content here instead of docs/docs/. - It is fine to do structural/content feedback here (clarity, examples, etc).
Files:
docs/draft/docs/servers/tools.mdx
docs/**
⚙️ CodeRabbit configuration file
docs/**: Repository documentation for the SDK, using MDX and hosted by Mintlify. See more specific rules for: - docs/docs/** (latest rendered docs, automation-only) - docs/v/** (archived versions, read-only) - docs/draft/docs/** (human-editable drafts) - docs/blogs/** (blogs, human edited) - docs/docs.json (Mintlify navigation)
Files:
docs/draft/docs/servers/tools.mdx
🧠 Learnings (13)
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Use `getCapabilities()` for dynamic capability exposure in registries instead of hardcoding capabilities in adapters
Applied to files:
libs/sdk/src/tool/tool.registry.tslibs/sdk/src/__test-utils__/mocks/prompt-registry.mock.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to **/index.ts : Export barrel exports in index.ts with clean, direct exports; do not add legacy or backwards compatibility exports
Applied to files:
apps/demo/src/apps/expenses/resources/index.tsapps/demo/src/apps/expenses/prompts/index.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/src/**/*.{ts,tsx} : Centralize record types in common/records directory and import from there rather than module-specific files
Applied to files:
apps/demo/src/apps/expenses/resources/index.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Do not mutate rawInput in flows - use state.set() for flow state management instead
Applied to files:
libs/sdk/src/prompt/flows/get-prompt.flow.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Use specific error classes extending PublicMcpError with MCP error codes instead of generic errors
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to **/*.{ts,tsx} : Use strict TypeScript mode with no `any` types without strong justification; prefer `unknown` for generic type defaults
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Use type parameters with constraints instead of unconstrained or `any` defaults in generic types
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Fail fast on invalid hook flows by validating hooks match their entry type during initialization
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to **/*.test.{ts,tsx} : Test constructor validation, error class instanceof checks, and all code paths including error conditions
Applied to files:
CLAUDE.mdlibs/sdk/src/common/schemas/annotated-class.schema.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Create shared base classes for common functionality (e.g., ExecutionContextBase) to reduce code duplication
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to **/*.{ts,tsx} : Avoid non-null assertions (`!`) - use proper error handling and throw specific error classes instead
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-26T15:23:04.938Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.938Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Every component must have a Zod schema with `.strict()` mode to reject unknown properties
Applied to files:
CLAUDE.mdlibs/sdk/src/common/schemas/annotated-class.schema.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Validate URIs per RFC 3986 at metadata level using Zod schema validation
Applied to files:
libs/sdk/src/common/schemas/annotated-class.schema.ts
🧬 Code graph analysis (3)
libs/sdk/src/prompt/flows/get-prompt.flow.ts (1)
libs/sdk/src/errors/mcp.error.ts (5)
InvalidInputError(223-240)InvalidMethodError(269-273)PromptNotFoundError(368-372)PromptExecutionError(377-396)InvalidOutputError(245-264)
libs/sdk/src/__test-utils__/mocks/prompt-registry.mock.ts (3)
libs/sdk/src/app/instances/app.local.instance.ts (1)
prompts(73-75)libs/sdk/src/scope/scope.instance.ts (1)
prompts(165-167)libs/sdk/src/common/interfaces/prompt.interface.ts (3)
output(91-93)output(95-98)error(111-113)
libs/sdk/src/common/schemas/annotated-class.schema.ts (2)
libs/sdk/src/common/tokens/tool.tokens.ts (1)
FrontMcpToolTokens(5-17)libs/sdk/src/common/tokens/resource.tokens.ts (2)
FrontMcpResourceTokens(5-14)FrontMcpResourceTemplateTokens(16-25)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (7)
libs/sdk/src/common/schemas/annotated-class.schema.ts (2)
81-99: LGTM! Comprehensive resource validation.The expanded validation correctly handles both Resource and ResourceTemplate forms, supporting both decorator-based and builder-style patterns. The inline comments and updated error message clearly document the accepted forms.
101-112: LGTM! Clear prompt validation logic.The validation correctly handles both decorator-based and builder-style patterns for prompts. The inline comments and updated error message make the accepted forms explicit.
CLAUDE.md (1)
128-311: SDK guideline updates are consistent and helpfulThe added guidance on MCP response types, validation flows, generic defaults (
unknown), registry capabilities, and anti-patterns matches the existing coding guidelines and the new prompt/tool flows in this PR. No changes needed.apps/demo/src/apps/expenses/prompts/categorize-expense.prompt.ts (1)
1-62: Function-style prompt definition is clean and consistentThe prompt configuration, argument handling, and returned message structure are clear and consistent with the rest of the demo prompts. No issues from a correctness or style perspective.
apps/demo/src/apps/expenses/index.ts (1)
11-40: Expenses app wiring for prompts/resources looks correctThe new prompts and resources are imported via their barrels and correctly registered on the
@Appconfig. This keeps the app configuration consistent and declarative.apps/demo/src/apps/expenses/resources/index.ts (1)
1-3: Barrel exports are clean and aligned with guidelinesDirectly re-exporting the default resources under descriptive names matches the
index.tsbarrel export guidance and keeps imports tidy.apps/demo/src/apps/expenses/prompts/index.ts (1)
1-2: Barrel exports look clean and follow the index.ts guidelineRe-exporting the two prompt modules as named exports is clear and matches the preferred barrel style for
index.ts.Based on learnings, this aligns with the
index.tsexport guidance.
…e error messages in schemas
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/sdk/src/common/schemas/annotated-class.schema.ts (1)
81-99: Resource schema correctly unifies Resource and ResourceTemplate decorators and builders
annotatedFrontMcpResourcesSchemanow:
- Accepts classes decorated with either
FrontMcpResourceTokens.typeorFrontMcpResourceTemplateTokens.type.- Accepts function-style builders that attach those token keys as properties (for
resource()andresourceTemplate()).The predicate is guarded with
typeof v === 'function'before accessing token keys, so it’s safe, and the updated error message clearly explains the accepted forms. If you also expose any@FrontMcpResource*aliases, you might optionally mention them in the message for consistency with providers/auth/providers/plugins/adapters, but the current wording is already accurate.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/demo/src/apps/expenses/resources/expense-policy.resource.ts(1 hunks)libs/sdk/src/__test-utils__/mocks/prompt-registry.mock.ts(1 hunks)libs/sdk/src/common/schemas/annotated-class.schema.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/sdk/src/test-utils/mocks/prompt-registry.mock.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with noanytypes without strong justification; preferunknownfor generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific error classes instead
Files:
libs/sdk/src/common/schemas/annotated-class.schema.tsapps/demo/src/apps/expenses/resources/expense-policy.resource.ts
libs/sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/**/*.{ts,tsx}: Create shared base classes for common functionality (e.g., ExecutionContextBase) to reduce code duplication
Use specific error classes extending PublicMcpError with MCP error codes instead of generic errors
UsegetCapabilities()for dynamic capability exposure in registries instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopein change event types to avoid confusion with the Scope class
Validate URIs per RFC 3986 at metadata level using Zod schema validation
Fail fast on invalid hook flows by validating hooks match their entry type during initialization
Use type parameters with constraints instead of unconstrained oranydefaults in generic types
Do not mutate rawInput in flows - use state.set() for flow state management instead
Files:
libs/sdk/src/common/schemas/annotated-class.schema.ts
libs/sdk/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Centralize record types in common/records directory and import from there rather than module-specific files
Files:
libs/sdk/src/common/schemas/annotated-class.schema.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/schemas/annotated-class.schema.ts
apps/demo/**
⚙️ CodeRabbit configuration file
apps/demo/**: apps/demo directory contains a demo application for testing purposes. It can be used as a reference for SDK usage examples.
Files:
apps/demo/src/apps/expenses/resources/expense-policy.resource.ts
🧠 Learnings (3)
📚 Learning: 2025-11-26T15:23:04.938Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.938Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Every component must have a Zod schema with `.strict()` mode to reject unknown properties
Applied to files:
libs/sdk/src/common/schemas/annotated-class.schema.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to libs/sdk/**/*.{ts,tsx} : Validate URIs per RFC 3986 at metadata level using Zod schema validation
Applied to files:
libs/sdk/src/common/schemas/annotated-class.schema.ts
📚 Learning: 2025-11-26T14:49:12.253Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T14:49:12.253Z
Learning: Applies to **/*.test.{ts,tsx} : Test constructor validation, error class instanceof checks, and all code paths including error conditions
Applied to files:
libs/sdk/src/common/schemas/annotated-class.schema.ts
🧬 Code graph analysis (1)
libs/sdk/src/common/schemas/annotated-class.schema.ts (2)
libs/sdk/src/common/tokens/tool.tokens.ts (1)
FrontMcpToolTokens(5-17)libs/sdk/src/common/tokens/resource.tokens.ts (2)
FrontMcpResourceTokens(5-14)FrontMcpResourceTemplateTokens(16-25)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (4)
apps/demo/src/apps/expenses/resources/expense-policy.resource.ts (1)
1-59: Expense policy resource is well-structured and demo-appropriateThe resource wiring and
executeimplementation look clean: static policy data is clearly organized, fully JSON-serializable, and returned in a singlecontentsentry withmimeType: 'application/json'. UsingNumber.MAX_SAFE_INTEGERfor the topapprovalLevelsthreshold avoids the earlierInfinity-serialization pitfall while remaining effectively “unbounded” for demo purposes. No correctness or typing issues stand out here.libs/sdk/src/common/schemas/annotated-class.schema.ts (3)
3-20: Import and metadata wiring for new resource template support looks correctThe addition of
FrontMcpResourceTemplateTokensto the tokens import and the extra metadata schema imports are consistent with the usages below (resources and auth/provider/plugin/adapters schemas). No issues from an API or typing perspective here.
71-79: Tool schema now correctly recognizes both decorator- and builder-style toolsThe
annotatedFrontMcpToolsSchemapredicate cleanly handles:
- Class/function tools annotated via
@Tool()/@FrontMcpTool()(metadata on the constructor), and- Function-style builders that set
FrontMcpToolTokens.typedirectly on the function object.The
typeof v === 'function'guard before metadata/property checks keeps this safe. No changes needed.
101-112: Prompt schema extension for builder-style prompts looks consistent
annotatedFrontMcpPromptsSchemanow validates both:
- Class-based prompts with
FrontMcpPromptTokensmetadata, and- Function-style
prompt()builders that attachFrontMcpPromptTokens.typeas a property.The
typeof v === 'function'guard ensures the property access is safe, and the error message clearly mentions both decorator and builder usage. This aligns well with the tools/resources handling.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.