-
Notifications
You must be signed in to change notification settings - Fork 4
feat(plugins): new CodeCall plugin #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a CodeCall plugin (search/describe/execute/invoke), AST validation & transformation (AgentScript) rules, a VM-based Enclave runtime, a TF-IDF vector DB (VectoriaDB), BaseConfig utility, provider/registry scope accessors, demo CRM app and many tests and docs; also updates tooling, path aliases, and package deps. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Model
participant CodeCall as CodeCall Plugin
participant Search as SearchTool
participant Describe as DescribeTool
participant Execute as ExecuteTool
participant Validator as AST Guard
participant Transform as AgentScript Transformer
participant Enclave as Enclave Runtime
participant VectorDB as VectoriaDB
participant Tool as Tool Implementation
Model->>CodeCall: request discovery / run script
CodeCall->>Search: codecall:search(query)
Search->>VectorDB: search embeddings / tfidf
VectorDB-->>Search: results
Search-->>CodeCall: tool list
Model->>CodeCall: codecall:execute(script)
CodeCall->>Execute: forward script
Execute->>Validator: validate(script)
alt validation ok
Validator-->>Execute: valid
Execute->>Transform: transformAgentScript(script)
Transform-->>Execute: transformedCode
Execute->>Enclave: execute(transformedCode, env)
Enclave->>Tool: callTool(name,input) (via host)
Tool-->>Enclave: result
Enclave-->>Execute: execution result
Execute-->>CodeCall: ok / logs
else validation failed
Validator-->>Execute: issues
Execute-->>CodeCall: syntax_error / illegal_access
end
CodeCall-->>Model: final result
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Specific attention recommended:
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)docs/draft/docs/**⚙️ CodeRabbit configuration file
Files:
docs/**⚙️ CodeRabbit configuration file
Files:
⏰ 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)
🔇 Additional comments (7)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/plugins/src/cache/cache.plugin.ts (1)
21-56: Fix uninitializeddefaultTTLand tighten read‑side cache logic (and consider removing console logs)There are a few related issues here:
defaultTTLis never initialized but is used in both hooks
private readonly defaultTTL: number;is declared but not assigned in the constructor.- Both
willReadCacheandwillWriteCachecompute TTLs fromthis.defaultTTL, so at runtime TTL will beundefinedand your “default 1 day” behavior won’t actually apply.A minimal fix is to wire
defaultTTLfrom the merged options in the constructor:- private readonly defaultTTL: number; + private readonly defaultTTL: number; static defaultOptions: CachePluginOptions = { type: 'memory', }; options: CachePluginOptions; constructor(options: CachePluginOptions = CachePlugin.defaultOptions) { super(); - this.options = { - defaultTTL: 60 * 60 * 24, - ...options, - }; + const merged = { + defaultTTL: 60 * 60 * 24, + ...options, + } as CachePluginOptions & { defaultTTL: number }; + + this.options = merged; + this.defaultTTL = merged.defaultTTL; }
willReadCacheshould not rewrite the store when there’s no cached valueRight now, when
cache === true(or whencache.ttl && cache.slideWindow), you callsetValueeven ifcachedisundefined, which can result in unnecessary writes (and potentially “cache an undefined” behavior, depending on the store):- const cached = await redis.getValue(hash); - - if (cache == true || (cache.ttl && cache.slideWindow)) { - const ttl = cache === true ? this.defaultTTL : cache.ttl ?? this.defaultTTL; - await redis.setValue(hash, cached, ttl); - } + const cached = await redis.getValue(hash); + + if ( + cached !== undefined && + (cache === true || (typeof cache === 'object' && cache.ttl && cache.slideWindow)) + ) { + const ttl = cache === true ? this.defaultTTL : cache.ttl ?? this.defaultTTL; + await redis.setValue(hash, cached, ttl); + }This preserves your sliding‑TTL semantics while only extending TTL for actual hits.
- Console logging inside the plugin
Both
console.log('return from cache', …)andconsole.log('willWriteCache', …)are likely too noisy for a shared plugin inlibs/. Consider either removing them or routing through whatever logging abstraction@frontmcp/sdkexposes so callers can control verbosity.Also applies to: 58-96
🧹 Nitpick comments (11)
docs/blog/11-2025/mcp-run-out-of-socket.mdx (1)
7-485: Blog formatting/content updates look good (with one optional style nit)The MDX changes keep content, links, and code samples accurate while improving consistency (className quoting, images, columns, steps, tips). Only minor style nit: in the “Multi-app composition” bullets, “Sometimes …” starts three consecutive lines; you could vary that wording if you care about strict editorial style, but it’s not required.
apps/demo/src/apps/expenses/tools/add.tool.ts (1)
1-13: Expense AddTool behavior matches primitive output convention
executecorrectly returns a primitivenumberandoutputSchema: z.number()matches that return type. For consistency with the calculator demo tools (which useoutputSchema: 'number'), you might consider aligning on one style, but functionally this is fine.apps/demo/src/apps/employee-time/tools/get-employee-details.tool.ts (1)
1-82: Aggregation logic is correct; consider reusing week/month day rangesThe flat output shape and the way you aggregate
open,openSince, and today/week/month totals across sites all look consistent.You’re recomputing
weekDaysandmonthDaysfor every site, though; you can precompute them once to cut allocations and make the loop clearer:const weekStart = startOfWeekMs(now); const weekEnd = endOfWeekMs(now); const monthStart = startOfMonthMs(now); const monthEnd = endOfMonthMs(now); + const weekDays = eachDayRange(weekStart, weekEnd); + const monthDays = eachDayRange(monthStart, monthEnd); + const sites = input.siteId ? [input.siteId] : await dir.listSitesForEmployee(employeeId); @@ - // Sum week - const weekDays = eachDayRange(weekStart, weekEnd); for (const d of weekDays) { weekMs += await getEmployeeDayMsForSite(redis, siteId, employeeId, d); } - // Sum month - const monthDays = eachDayRange(monthStart, monthEnd); for (const d of monthDays) { monthMs += await getEmployeeDayMsForSite(redis, siteId, employeeId, d); }docs/draft/docs/plugins/codecall-plugin.mdx (1)
420-543: Minor wording/spelling nits in status/PII sectionsVery minor, but a couple of tweaks would improve polish:
- In the status list: change “blocked builtin/global” to “blocked built‑in/global”.
- In the PII section: “exactly what a normal tool call would see” and “outside of CodeCall” can be tightened slightly.
For example:
- - `illegal_access` – attempted to use a blocked builtin/global. + - `illegal_access` – attempted to use a blocked built-in/global. @@ -- The response that CodeCall sees is exactly what a normal tool call would see +- The response that CodeCall sees is the same as what a normal tool call would see @@ -- to calling the same tools one-by-one outside of CodeCall. +- to calling the same tools one-by-one outside CodeCall.Purely optional, but it addresses the style/spelling flags without changing meaning.
libs/sdk/src/common/decorators/tool.decorator.ts (1)
35-38: Consider relaxingOut extends objectto support primitive output schemas
FrontMcpToolExecuteHandler<In, Out>andfrontMcpToolboth constrainOuttoextends object, and the default forOutonly handlesZodRawShapeoutputs. With the newoutputSchemasupport for primitives like'number', this means function-style tools can’t naturally express primitive outputs in their typings (they’ll be typed asobjecteven if they returnnumber).If you want function tools to align with the new outputSchema semantics (as class-based
@Toolalready does), consider loosening the constraint and default, e.g.:-export type FrontMcpToolExecuteHandler<In extends object, Out extends object> = ( +export type FrontMcpToolExecuteHandler<In extends object, Out> = ( input: In, ctx: ToolContext<In, Out>, ) => Out | Promise<Out>; -function frontMcpTool< - T extends ToolMetadata, - In extends object = z.baseObjectInputType<T['inputSchema']>, - Out extends object = T['outputSchema'] extends z.ZodRawShape ? z.baseObjectInputType<T['outputSchema']> : object, ->(providedMetadata: T): (handler: FrontMcpToolExecuteHandler<In, Out>) => () => void { +function frontMcpTool< + T extends ToolMetadata, + In extends object = z.baseObjectInputType<T['inputSchema']>, + Out = T['outputSchema'] extends z.ZodRawShape ? z.baseObjectInputType<T['outputSchema']> : unknown, +>(providedMetadata: T): (handler: FrontMcpToolExecuteHandler<In, Out>) => () => void {This keeps existing structured cases working while allowing primitive output schemas for function tools.
Also applies to: 43-47
libs/plugins/src/codecall/README.md (1)
1-834: Excellent internal documentation for contributors.This README provides comprehensive coverage of the CodeCall architecture, including:
- High-level design and goals
- Detailed mode semantics and best practices
- VM security pipeline with CSP-like presets
- Meta-tool contracts and error models
- PII/privacy guarantees
- Development workflow and testing guidelines
The documentation will be valuable for contributors working on the CodeCall implementation.
Optional: The markdown linter flags some list indentation inconsistencies (e.g., lines 212-220, 625-640). While purely cosmetic, standardizing to 2-space indentation for nested lists would improve formatting consistency.
libs/plugins/src/codecall/tools/describe.schema.ts (1)
14-22: Consider if passthrough schemas are appropriate for tool metadata.The
inputSchemaandoutputSchemafields usez.object({}).passthrough(), which accepts any properties without validation. While this provides flexibility for describing arbitrary tools, it sacrifices type safety and validation.If tool schemas follow a known structure (e.g., Zod schemas serialized to JSON Schema), consider using a more specific schema like
z.record(z.unknown())or referencing the JSON Schema specification.apps/demo/src/apps/employee-time/tools/register-exit.tool.ts (1)
12-12: Align output schema style with other employee-time toolsLogic and types are sound, and the flat
{ employeeId, endedAt, durationMs }response matches the admin tools. The only nit is thatoutputSchemahere is a raw Zod shape, while sibling tools (e.g.,admin-add-entry,admin-add-exit) usez.object({ ... }). Consider standardizing on one style for consistency and to keep downstream consumers’ expectations simple.Also applies to: 46-46
apps/demo/src/apps/employee-time/tools/admin-add-exit.tool.ts (1)
12-16: Admin flag enforced only at schema levelThe schema-level
admin: z.boolean().refine((v) => v === true, ...)is doing all the access control work; theexecutemethod itself never inspectsinput.admin. That’s fine if the Tool decorator always validates inputs before execution, but if direct programmatic calls are possible, you may want a defensive runtime check as well.Also applies to: 17-21, 24-55
libs/plugins/src/codecall/codecall.plugin.ts (1)
23-44: Ensure CodeCall config seen via CodeCallConfig matches merged defaultsThe constructor nicely merges
CodeCallPlugin.defaultOptionswith user options (including nestedvm), butdynamicProviderscurrently exposes the rawoptionsargument asCodeCallConfig. If the host does not pre-merge defaults, the values seen by providers (e.g., the VM runner) can diverge fromthis.options, and theresolveVmOptionshelper remains unused.Consider either:
- having the plugin system pass already-merged options into
dynamicProviders, or- applying the same default merge (and possibly
resolveVmOptions) insidedynamicProvidersbefore exposingCodeCallConfig.That keeps VM behavior predictable and avoids different default semantics between plugin internals and consumers.
Also applies to: 50-60, 80-158
libs/plugins/src/codecall/tools/execute.schema.ts (1)
7-90: Execute result schema is well-structured; minor nit onkindtypingThe discriminated union on
statuswith detailed payloads covers the main VM/tool failure modes cleanly, and the exported types will be easy to consume.One small nit: in
illegalAccessErrorPayloadSchema.kindyou union specific literals withz.string(), which effectively widens the field tostring. If the intent is just documentation that certain values are common, that’s fine; if you want consumers to be able to exhaustively switch on known kinds, consider restricting to a narrower set or separating akindvsdetailsfield.Also applies to: 96-109
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
docs/assets/banners/codecall-plugin/code-call-example.pngis excluded by!**/*.pngdocs/assets/banners/codecall-plugin/code-call-options.dark.pngis excluded by!**/*.pngdocs/assets/banners/codecall-plugin/code-call-options.light.pngis excluded by!**/*.pngdocs/assets/banners/codecall-plugin/code-call-plugin.pngis excluded by!**/*.pngdocs/assets/banners/codecall-plugin/early-day-vs-today.dark.pngis excluded by!**/*.pngdocs/assets/banners/codecall-plugin/early-day-vs-today.light.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (59)
.prettierrc(1 hunks)apps/demo/src/apps/calculator/tools/abs.tool.ts(1 hunks)apps/demo/src/apps/calculator/tools/add.tool.ts(1 hunks)apps/demo/src/apps/calculator/tools/ceil.tool.ts(1 hunks)apps/demo/src/apps/calculator/tools/divide.tool.ts(1 hunks)apps/demo/src/apps/calculator/tools/exp.tool.ts(1 hunks)apps/demo/src/apps/calculator/tools/floor.tool.ts(1 hunks)apps/demo/src/apps/calculator/tools/max.tool.ts(1 hunks)apps/demo/src/apps/calculator/tools/min.tool.ts(1 hunks)apps/demo/src/apps/calculator/tools/modulo.tool.ts(1 hunks)apps/demo/src/apps/calculator/tools/multiply.tool.ts(1 hunks)apps/demo/src/apps/calculator/tools/pow.tool.ts(1 hunks)apps/demo/src/apps/calculator/tools/round.tool.ts(2 hunks)apps/demo/src/apps/calculator/tools/sqrt.tool.ts(1 hunks)apps/demo/src/apps/calculator/tools/subtract.tool.ts(1 hunks)apps/demo/src/apps/employee-time/index.ts(1 hunks)apps/demo/src/apps/employee-time/plugins/site-authorization.plugin.ts(4 hunks)apps/demo/src/apps/employee-time/providers/employee-directory.provider.ts(3 hunks)apps/demo/src/apps/employee-time/tools/admin-add-entry.tool.ts(2 hunks)apps/demo/src/apps/employee-time/tools/admin-add-exit.tool.ts(2 hunks)apps/demo/src/apps/employee-time/tools/get-employee-details.tool.ts(3 hunks)apps/demo/src/apps/employee-time/tools/list-employees.tool.ts(2 hunks)apps/demo/src/apps/employee-time/tools/register-exit.tool.ts(2 hunks)apps/demo/src/apps/employee-time/tools/report-hours.tool.ts(0 hunks)apps/demo/src/apps/expenses/index.ts(1 hunks)apps/demo/src/apps/expenses/tools/add.tool.ts(1 hunks)apps/demo/src/apps/expenses/tools/create-expense.tool.ts(2 hunks)apps/demo/src/main.ts(1 hunks)docs/blog/11-2025/codecall-plugin.mdx(1 hunks)docs/blog/11-2025/introducing-frontmcp.mdx(12 hunks)docs/blog/11-2025/mcp-run-out-of-socket.mdx(10 hunks)docs/blog/external-links.mdx(1 hunks)docs/blog/index.mdx(3 hunks)docs/blog/style.css(1 hunks)docs/docs.json(7 hunks)docs/draft/docs/plugins/codecall-plugin.mdx(1 hunks)docs/snippets/card.jsx(1 hunks)libs/plugins/package.json(1 hunks)libs/plugins/src/cache/cache.plugin.ts(5 hunks)libs/plugins/src/cache/cache.symbol.ts(1 hunks)libs/plugins/src/cache/cache.types.ts(1 hunks)libs/plugins/src/codecall/README.md(1 hunks)libs/plugins/src/codecall/codecall.plugin.ts(1 hunks)libs/plugins/src/codecall/codecall.symbol.ts(1 hunks)libs/plugins/src/codecall/codecall.types.ts(1 hunks)libs/plugins/src/codecall/index.ts(1 hunks)libs/plugins/src/codecall/providers/codecall-ast-validator.provider.ts(1 hunks)libs/plugins/src/codecall/providers/codecall-vm2.provider.ts(1 hunks)libs/plugins/src/codecall/tools/describe.schema.ts(1 hunks)libs/plugins/src/codecall/tools/describe.tool.ts(1 hunks)libs/plugins/src/codecall/tools/execute.schema.ts(1 hunks)libs/plugins/src/codecall/tools/execute.tool.ts(1 hunks)libs/plugins/src/codecall/tools/index.ts(1 hunks)libs/plugins/src/codecall/tools/invoke.schema.ts(1 hunks)libs/plugins/src/codecall/tools/invoke.tool.ts(1 hunks)libs/plugins/src/codecall/tools/search.schema.ts(1 hunks)libs/plugins/src/codecall/tools/search.tool.ts(1 hunks)libs/sdk/src/common/decorators/tool.decorator.ts(3 hunks)libs/sdk/src/common/metadata/tool.metadata.ts(3 hunks)
💤 Files with no reviewable changes (1)
- apps/demo/src/apps/employee-time/tools/report-hours.tool.ts
🧰 Additional context used
📓 Path-based instructions (5)
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/plugins/src/cache/cache.symbol.tslibs/plugins/src/codecall/tools/execute.tool.tslibs/plugins/src/codecall/tools/invoke.tool.tslibs/plugins/src/codecall/tools/describe.tool.tslibs/plugins/src/codecall/tools/search.tool.tslibs/plugins/src/codecall/tools/describe.schema.tslibs/plugins/src/cache/cache.types.tslibs/plugins/src/codecall/providers/codecall-vm2.provider.tslibs/plugins/package.jsonlibs/plugins/src/codecall/tools/execute.schema.tslibs/plugins/src/codecall/codecall.plugin.tslibs/plugins/src/codecall/tools/index.tslibs/plugins/src/codecall/codecall.types.tslibs/plugins/src/codecall/index.tslibs/plugins/src/codecall/README.mdlibs/plugins/src/codecall/codecall.symbol.tslibs/plugins/src/codecall/providers/codecall-ast-validator.provider.tslibs/plugins/src/codecall/tools/search.schema.tslibs/plugins/src/codecall/tools/invoke.schema.tslibs/sdk/src/common/decorators/tool.decorator.tslibs/sdk/src/common/metadata/tool.metadata.tslibs/plugins/src/cache/cache.plugin.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/calculator/tools/max.tool.tsapps/demo/src/apps/calculator/tools/floor.tool.tsapps/demo/src/apps/employee-time/index.tsapps/demo/src/apps/calculator/tools/divide.tool.tsapps/demo/src/apps/expenses/tools/add.tool.tsapps/demo/src/apps/calculator/tools/pow.tool.tsapps/demo/src/apps/calculator/tools/exp.tool.tsapps/demo/src/apps/calculator/tools/add.tool.tsapps/demo/src/apps/calculator/tools/ceil.tool.tsapps/demo/src/apps/calculator/tools/round.tool.tsapps/demo/src/apps/calculator/tools/modulo.tool.tsapps/demo/src/apps/calculator/tools/abs.tool.tsapps/demo/src/apps/calculator/tools/min.tool.tsapps/demo/src/apps/employee-time/tools/list-employees.tool.tsapps/demo/src/apps/employee-time/tools/register-exit.tool.tsapps/demo/src/apps/expenses/tools/create-expense.tool.tsapps/demo/src/main.tsapps/demo/src/apps/employee-time/providers/employee-directory.provider.tsapps/demo/src/apps/employee-time/tools/admin-add-entry.tool.tsapps/demo/src/apps/calculator/tools/multiply.tool.tsapps/demo/src/apps/calculator/tools/sqrt.tool.tsapps/demo/src/apps/employee-time/tools/get-employee-details.tool.tsapps/demo/src/apps/expenses/index.tsapps/demo/src/apps/calculator/tools/subtract.tool.tsapps/demo/src/apps/employee-time/tools/admin-add-exit.tool.tsapps/demo/src/apps/employee-time/plugins/site-authorization.plugin.ts
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/blog/style.cssdocs/blog/11-2025/codecall-plugin.mdxdocs/draft/docs/plugins/codecall-plugin.mdxdocs/docs.jsondocs/snippets/card.jsxdocs/blog/index.mdxdocs/blog/external-links.mdxdocs/blog/11-2025/mcp-run-out-of-socket.mdxdocs/blog/11-2025/introducing-frontmcp.mdx
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/plugins/codecall-plugin.mdx
docs/docs.json
⚙️ CodeRabbit configuration file
docs/docs.json: Mintlify navigation configuration. Important structure: - dropdowns[].dropdown == "Documentation":
- Contains a "versions" array.
- Latest version entry: "version": "vX.Y (latest)", "default": true, with
paths like "docs/getting-started/welcome" and "updates" (NO "v/X.Y" prefix).- Older versions: "version": "v0.2", "version": "v0.1", etc., with paths
like "docs/v/0.2/getting-started/welcome".- dropdowns[].dropdown == "Blog":
- Contains blog groups (e.g. "blog/november-2025/introducing-frontmcp").
Review guidelines: - Do NOT suggest editing the Blog dropdown unless the PR explicitly adds/changes blogs. - Ensure latest version uses non-versioned "docs/..." paths, and older versions
use "docs/v/{minor}/..." paths.- If you see changes that try to route archived versions away from docs/v/**,
flag that as incorrect.
Files:
docs/docs.json
🧬 Code graph analysis (24)
libs/plugins/src/codecall/tools/execute.tool.ts (2)
apps/demo/src/apps/calculator/tools/add.tool.ts (1)
Tool(4-14)libs/plugins/src/codecall/tools/execute.schema.ts (2)
codeCallExecuteResultSchema(83-90)CodeCallExecuteResult(103-109)
apps/demo/src/apps/employee-time/index.ts (2)
apps/demo/src/apps/employee-time/providers/redis.provider.ts (1)
createEmployeeRedisProvider(90-96)apps/demo/src/apps/employee-time/providers/employee-directory.provider.ts (1)
createEmployeeDirectoryProvider(64-70)
apps/demo/src/apps/expenses/tools/add.tool.ts (3)
apps/demo/src/apps/calculator/tools/abs.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/add.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/multiply.tool.ts (1)
Tool(4-14)
libs/plugins/src/codecall/tools/invoke.tool.ts (1)
libs/plugins/src/codecall/tools/invoke.schema.ts (4)
invokeToolInputSchema(4-6)invokeToolOutputSchema(10-10)InvokeToolInput(8-8)InvokeToolOutput(11-11)
apps/demo/src/apps/calculator/tools/pow.tool.ts (2)
apps/demo/src/apps/calculator/tools/abs.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/add.tool.ts (1)
Tool(4-14)
libs/plugins/src/codecall/tools/describe.tool.ts (1)
libs/plugins/src/codecall/tools/describe.schema.ts (4)
describeToolInputSchema(5-10)describeToolOutputSchema(14-22)DescribeToolInput(12-12)DescribeToolOutput(23-23)
libs/plugins/src/codecall/tools/search.tool.ts (1)
libs/plugins/src/codecall/tools/search.schema.ts (4)
searchToolInputSchema(4-6)searchToolOutputSchema(10-10)SearchToolInput(8-8)SearchToolOutput(11-11)
libs/plugins/src/codecall/providers/codecall-vm2.provider.ts (4)
libs/plugins/src/codecall/providers/codecall-ast-validator.provider.ts (1)
Provider(14-91)libs/plugins/src/codecall/codecall.symbol.ts (2)
ResolvedCodeCallVmOptions(24-32)CodeCallVmEnvironment(46-65)libs/plugins/src/codecall/codecall.types.ts (1)
CodeCallExecuteInput(123-129)libs/plugins/src/codecall/tools/execute.schema.ts (1)
CodeCallExecuteResult(103-109)
libs/plugins/src/codecall/codecall.plugin.ts (3)
libs/plugins/src/cache/cache.plugin.ts (1)
Plugin(7-97)libs/plugins/src/codecall/codecall.symbol.ts (3)
CodeCallAstValidator(34-40)CodeCallConfig(6-6)ResolvedCodeCallVmOptions(24-32)libs/plugins/src/codecall/codecall.types.ts (3)
CodeCallPluginOptions(42-57)CodeCallVmOptions(7-20)CodeCallVmPreset(5-5)
apps/demo/src/apps/calculator/tools/abs.tool.ts (13)
apps/demo/src/apps/calculator/tools/add.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/ceil.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/divide.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/exp.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/floor.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/max.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/min.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/modulo.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/multiply.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/pow.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/round.tool.ts (1)
Tool(4-22)apps/demo/src/apps/calculator/tools/sqrt.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/subtract.tool.ts (1)
Tool(4-14)
libs/plugins/src/codecall/codecall.symbol.ts (1)
libs/plugins/src/codecall/codecall.types.ts (2)
CodeCallPluginOptions(42-57)CodeCallVmPreset(5-5)
apps/demo/src/apps/employee-time/tools/list-employees.tool.ts (1)
apps/demo/src/apps/employee-time/providers/employee-directory.provider.ts (1)
EmployeeDirectoryProvider(5-62)
apps/demo/src/apps/employee-time/tools/register-exit.tool.ts (3)
apps/demo/src/apps/employee-time/tools/admin-add-entry.tool.ts (1)
Tool(8-40)apps/demo/src/apps/employee-time/tools/admin-add-exit.tool.ts (1)
Tool(8-57)apps/demo/src/apps/employee-time/utils/keys.ts (2)
sessionsKey(22-24)openKey(14-16)
apps/demo/src/apps/expenses/tools/create-expense.tool.ts (1)
apps/demo/src/apps/expenses/tools/add.tool.ts (1)
Tool(4-14)
libs/plugins/src/codecall/providers/codecall-ast-validator.provider.ts (2)
libs/plugins/src/codecall/providers/codecall-vm2.provider.ts (1)
Provider(10-84)libs/plugins/src/codecall/codecall.symbol.ts (4)
CodeCallAstValidator(34-40)ResolvedCodeCallVmOptions(24-32)CodeCallAstValidationResult(15-18)CodeCallAstValidationIssue(8-13)
apps/demo/src/apps/employee-time/tools/admin-add-entry.tool.ts (2)
apps/demo/src/apps/employee-time/tools/admin-add-exit.tool.ts (1)
Tool(8-57)apps/demo/src/apps/employee-time/tools/get-employee-details.tool.ts (1)
Tool(9-84)
apps/demo/src/apps/calculator/tools/multiply.tool.ts (4)
apps/demo/src/apps/calculator/tools/abs.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/add.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/pow.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/subtract.tool.ts (1)
Tool(4-14)
apps/demo/src/apps/calculator/tools/sqrt.tool.ts (2)
apps/demo/src/apps/calculator/tools/abs.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/add.tool.ts (1)
Tool(4-14)
apps/demo/src/apps/employee-time/tools/get-employee-details.tool.ts (2)
apps/demo/src/apps/employee-time/tools/admin-add-entry.tool.ts (1)
Tool(8-40)apps/demo/src/apps/employee-time/tools/admin-add-exit.tool.ts (1)
Tool(8-57)
apps/demo/src/apps/expenses/index.ts (2)
apps/demo/src/apps/employee-time/index.ts (1)
App(14-34)apps/demo/src/apps/calculator/index.ts (1)
App(19-50)
apps/demo/src/apps/calculator/tools/subtract.tool.ts (4)
apps/demo/src/apps/calculator/tools/abs.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/add.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/divide.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/multiply.tool.ts (1)
Tool(4-14)
apps/demo/src/apps/employee-time/tools/admin-add-exit.tool.ts (2)
apps/demo/src/apps/employee-time/tools/admin-add-entry.tool.ts (1)
Tool(8-40)apps/demo/src/apps/employee-time/utils/keys.ts (2)
sessionsKey(22-24)openKey(14-16)
libs/sdk/src/common/decorators/tool.decorator.ts (3)
libs/sdk/src/common/metadata/tool.metadata.ts (5)
ToolMetadata(138-186)ImageOutputSchema(87-87)AudioOutputSchema(94-94)ResourceOutputSchema(101-101)ResourceLinkOutputSchema(108-108)apps/demo/src/apps/calculator/tools/abs.tool.ts (1)
Tool(4-14)apps/demo/src/apps/calculator/tools/add.tool.ts (1)
Tool(4-14)
libs/plugins/src/cache/cache.plugin.ts (1)
libs/plugins/src/cache/cache.symbol.ts (1)
CacheStoreToken(4-4)
🪛 ESLint
libs/sdk/src/common/decorators/tool.decorator.ts
[error] 188-188: The {} ("empty object") type allows any non-nullish value, including literals like 0 and "".
- If that's what you want, disable this lint rule with an inline comment or configure the 'allowObjectTypes' rule option.
- If you want a type meaning "any object", you probably want
objectinstead. - If you want a type meaning "any value", you probably want
unknowninstead.
(@typescript-eslint/no-empty-object-type)
[error] 195-195: The {} ("empty object") type allows any non-nullish value, including literals like 0 and "".
- If that's what you want, disable this lint rule with an inline comment or configure the 'allowObjectTypes' rule option.
- If you want a type meaning "any object", you probably want
objectinstead. - If you want a type meaning "any value", you probably want
unknowninstead.
(@typescript-eslint/no-empty-object-type)
[error] 202-202: The {} ("empty object") type allows any non-nullish value, including literals like 0 and "".
- If that's what you want, disable this lint rule with an inline comment or configure the 'allowObjectTypes' rule option.
- If you want a type meaning "any object", you probably want
objectinstead. - If you want a type meaning "any value", you probably want
unknowninstead.
(@typescript-eslint/no-empty-object-type)
[error] 218-218: The {} ("empty object") type allows any non-nullish value, including literals like 0 and "".
- If that's what you want, disable this lint rule with an inline comment or configure the 'allowObjectTypes' rule option.
- If you want a type meaning "any object", you probably want
objectinstead. - If you want a type meaning "any value", you probably want
unknowninstead.
(@typescript-eslint/no-empty-object-type)
[error] 221-221: The {} ("empty object") type allows any non-nullish value, including literals like 0 and "".
- If that's what you want, disable this lint rule with an inline comment or configure the 'allowObjectTypes' rule option.
- If you want a type meaning "any object", you probably want
objectinstead. - If you want a type meaning "any value", you probably want
unknowninstead.
(@typescript-eslint/no-empty-object-type)
libs/sdk/src/common/metadata/tool.metadata.ts
[error] 15-15: An empty interface declaration allows any non-nullish value, including literals like 0 and "".
- If that's what you want, disable this lint rule with an inline comment or configure the 'allowInterfaces' rule option.
- If you want a type meaning "any object", you probably want
objectinstead. - If you want a type meaning "any value", you probably want
unknowninstead.
(@typescript-eslint/no-empty-object-type)
[error] 15-15: An empty interface is equivalent to {}.
(@typescript-eslint/no-empty-interface)
🪛 LanguageTool
docs/blog/11-2025/codecall-plugin.mdx
[style] ~50-~50: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd inherit 50–100 endpoints as tools. - You introduce per-tenant tools, or vari...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~51-~51: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..., or variants for different products. - You start experimenting with plugins an...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~162-~162: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... list_tools or your context window. - You want the model to: - Combine tools int...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~525-~525: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...mal MCP call: - PII plugin(s) - auth - rate limiting - logging/audit This is perfect for: ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~532-~532: It seems that a subject is missing.
Context: ...ency when orchestration isn’t needed. - Still leveraging CodeCall’s **modes, allowlis...
(SENT_START_STILL_VBG)
[style] ~556-~556: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n** still enforces identities/scopes. - Your logging/rate-limit plugins still ru...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
docs/draft/docs/plugins/codecall-plugin.mdx
[style] ~38-~38: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...t to dump them all into list_tools. - You want the LLM to: - Combine multiple too...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~141-~141: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... expose all of them via CodeCall. - You don’t want to list them all. **Beh...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~175-~175: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...calls and CodeCall on the same tools. - You’re happy to configure everything per-to...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~422-~422: Ensure spelling is correct
Context: ...al_access– attempted to use a blocked builtin/global. -runtime_error` – bug in the ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~535-~535: Consider an alternative for the overused word “exactly”.
Context: .... - The response that CodeCall sees is exactly what a normal tool call would see aft...
(EXACTLY_PRECISELY)
[style] ~543-~543: This phrase is redundant. Consider using “outside”.
Context: ... to calling the same tools one-by-one outside of CodeCall. --- ## Tool discovery, cach...
(OUTSIDE_OF)
libs/plugins/src/codecall/README.md
[style] ~119-~119: Consider an alternative for the overused word “exactly”.
Context: ...a paths: - The data seen by the VM is exactly what a normal tool call would see *...
(EXACTLY_PRECISELY)
[style] ~122-~122: This phrase is redundant. Consider using “outside”.
Context: ...o calling the same tools one-by-one outside of CodeCall. For PII specifically: - A P...
(OUTSIDE_OF)
[style] ~132-~132: ‘in control of’ might be wordy. Consider a shorter alternative.
Context: ...redaction/obfuscation logic stays fully in control of the PII plugin. In other words: CodeCa...
(EN_WORDINESS_PREMIUM_IN_CONTROL_OF)
docs/blog/external-links.mdx
[uncategorized] ~12-~12: The official name of this software platform is spelled with a capital “H”.
Context: ...={'GitHub Repo'} horizontal arrow icon="github"> FrontMCP source code on Github </Ca...
(GITHUB)
[uncategorized] ~13-~13: The official name of this software platform is spelled with a capital “H”.
Context: ...con="github"> FrontMCP source code on Github <Card href={'/updates'} title={...
(GITHUB)
docs/blog/11-2025/mcp-run-out-of-socket.mdx
[style] ~130-~130: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...with separate auth and base paths. - Sometimes expose an app as a standalone MCP serve...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.18.1)
libs/plugins/src/codecall/README.md
212-212: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
213-213: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
216-216: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
217-217: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
219-219: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
220-220: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
233-233: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
234-234: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
237-237: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
251-251: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
254-254: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
625-625: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
627-627: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
628-628: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
629-629: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
633-633: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
635-635: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
636-636: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
637-637: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
638-638: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
639-639: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
640-640: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
673-673: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
674-674: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
675-675: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
678-678: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
679-679: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
680-680: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
682-682: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
683-683: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
684-684: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
713-713: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
714-714: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
715-715: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
716-716: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
723-723: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
726-726: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
728-728: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
729-729: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
731-731: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
733-733: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
734-734: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
735-735: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
736-736: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
746-746: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
747-747: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
748-748: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
752-752: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
792-792: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
795-795: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
796-796: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
797-797: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
798-798: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
799-799: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
802-802: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
805-805: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
808-808: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
809-809: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
810-810: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
824-824: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
825-825: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
826-826: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (43)
.prettierrc (1)
23-26: Formatting change—no functional impact.The conversion of the
filesarray from single-line to multi-line format is a pure stylistic update that does not affect Prettier's configuration behavior. The JSON remains valid and semantically equivalent.apps/demo/src/apps/employee-time/plugins/site-authorization.plugin.ts (1)
1-87: No issues identified; formatting-only changes.This file contains purely formatting and whitespace adjustments with no functional, logic, or behavioral changes. As per review guidelines, formatting issues are not flagged without static analysis tool indication.
libs/plugins/src/cache/cache.symbol.ts (1)
1-4: CacheStoreToken import and export look correctImports and the typed
CacheStoreTokenexport remain consistent with existing cache types; no behavioral change introduced.libs/plugins/src/cache/cache.types.ts (1)
1-59: Cache type definitions align with plugin usage
CachePluginToolOptions,CachePluginOptions, andCacheStoreInterfaceare coherent and match how the cache plugin consumesoptions.type,defaultTTL, and tool-levelmetadata.cache.apps/demo/src/main.ts (1)
1-15: Demo server wiring is consistent with FrontMCP patternsServer configuration (apps list,
splitByApp: true, verbose logging, HTTP port) is straightforward and consistent with the docs/examples.docs/docs.json (1)
10-310: docs.json versioning and nav structure look correct
- Latest docs (
v0.3 (latest)) use unversioneddocs/...paths, whilev0.2andv0.1correctly usedocs/v/{minor}/....- New
"main (draft)"version is hidden, non-default, and routes todraft/docs/..., which matches the “draft” convention.- New plugin draft pages and the
blog/11-2025/codecall-pluginentry in the Blog dropdown are wired consistently with the new content.All of this aligns with the documented navigation/versioning rules.
Also applies to: 318-327
docs/blog/style.css (1)
1-80: Blog card + light/dark styles look consistentFixed card widths, light/dark image toggling, and responsive overrides are coherent and align with the new
light-img/dark-imgusage; no issues from a layout or theming perspective.docs/blog/11-2025/introducing-frontmcp.mdx (1)
7-321: MDX structure and content updates look goodLight/dark banner images align with the new CSS, the example TypeScript remains valid, and the structural MDX (Columns, Card, Tip, link refs) is consistent; no issues from a docs/rendering perspective.
libs/sdk/src/common/decorators/tool.decorator.ts (2)
4-12: New metadata and ToolContext imports look correctThe added imports for
ImageOutputSchema,AudioOutputSchema,ResourceOutputSchema,ResourceLinkOutputSchema, andToolContextline up with the metadata definitions you’re using later in the module augmentation; no issues here.Also applies to: 14-14
63-166: Output inference helpers (__OutputOf,__OutputSchema) are consistent with new tool patternsThe new Zod/output inference helpers (
__InferZod,__InferFromSingleSchema,__InferFromArraySchema,__OutputOf,__OutputSchema,__ToolOptions) correctly:
- Map MCP literals (
'image' | 'audio' | 'resource' | 'resource_link') to their Zod-backed types.- Map primitive literals (
'string' | 'number' | 'boolean' | 'date') to TS primitives /Date.- Handle both single and array schemas for
outputSchema.- Drive the
Tooldecorator overloads so class-based tools like the calculator examples get accurateexecutereturn types.This looks sound and should give much better type feedback at call sites.
Also applies to: 237-265
apps/demo/src/apps/employee-time/providers/employee-directory.provider.ts (1)
3-3: Provider API remains consistent with tools and behaviorThe added
keysimport and the explicitlistEmployeesoptions type/return type align with existing usage (list-employees tool) and keep behavior unchanged, including the search filter. Nothing to fix here.Also applies to: 31-36, 55-55
libs/plugins/src/codecall/index.ts (1)
1-2: CodeCall plugin barrel exports look appropriateRe-exporting the default
codecall.pluginandcodecall.typeshere gives a clean entry point for@frontmcp/plugins/codecall. Just ensure the top-level@frontmcp/pluginsbarrel also re-exports this module if that’s part of the intended public API.apps/demo/src/apps/employee-time/index.ts (1)
1-4: Employee-time app wiring to Redis providers and CachePlugin is coherentUsing
createEmployeeRedisProviderandcreateEmployeeDirectoryProviderinproviders, plusCachePlugin.init({ type: 'redis', config: { host: 'localhost', port: 6379 } }), keeps the Redis config consistent across DI and caching. The App surface remains clear and backward-compatible aside from the removed tool.Also applies to: 17-24
docs/blog/index.mdx (1)
10-10: Blog index updates correctly surface the new CodeCall article and dark-mode bannerThe new CodeCall
BlogCardentry and the addition ofimgDarkfor the “Introducing FrontMCP” post match the expected BlogCard API and improve dark-mode theming. Links and metadata look coherent.Also applies to: 24-35, 49-57
apps/demo/src/apps/calculator/tools/divide.tool.ts (1)
8-12: LGTM - Output normalization applied consistently.The tool correctly returns a primitive number and the outputSchema reflects this change. The division-by-zero validation in the inputSchema provides proper error handling.
apps/demo/src/apps/expenses/tools/create-expense.tool.ts (1)
24-24: LGTM - Improved output value.Changed from placeholder 'asdasdsd' to meaningful 'OK' value, improving code quality.
apps/demo/src/apps/expenses/index.ts (1)
1-39: LGTM - Formatting improvements.Consistent code style improvements with proper spacing and trailing commas throughout the App configuration.
apps/demo/src/apps/calculator/tools/add.tool.ts (1)
8-12: LGTM - Output normalization applied consistently.The tool correctly returns a primitive number matching the updated outputSchema.
apps/demo/src/apps/calculator/tools/max.tool.ts (1)
8-12: LGTM - Output normalization applied consistently.The tool correctly returns a primitive number using Math.max with proper array spreading.
apps/demo/src/apps/calculator/tools/floor.tool.ts (1)
8-12: LGTM - Output normalization applied consistently.The tool correctly returns a primitive number using Math.floor.
apps/demo/src/apps/calculator/tools/modulo.tool.ts (1)
8-12: LGTM - Output normalization applied consistently.The tool correctly returns a primitive number with proper modulo-by-zero validation in the inputSchema.
libs/plugins/src/codecall/tools/index.ts (1)
1-4: LGTM!Clean re-export barrel file following standard module organization patterns.
libs/plugins/src/codecall/tools/search.tool.ts (1)
12-14: Stub implementation is appropriate for initial scaffolding.The placeholder return is acceptable for this PR as it establishes the tool interface. Ensure the actual search logic is implemented before release.
apps/demo/src/apps/calculator/tools/ceil.tool.ts (1)
8-12: LGTM!The output schema normalization from
{ result: number }to primitivenumbersimplifies the API and aligns with the broader changes across calculator tools.apps/demo/src/apps/calculator/tools/min.tool.ts (1)
8-12: LGTM!Consistent output schema normalization to primitive
number, matching the pattern applied across other calculator tools.apps/demo/src/apps/calculator/tools/exp.tool.ts (1)
8-12: LGTM!Output schema normalization to primitive
numberis consistent with the refactoring across calculator tools.libs/plugins/src/codecall/tools/execute.tool.ts (1)
7-16: LGTM!The tool definition is well-structured with consistent naming (
CodeCallExecuteToolforcodecall:execute), clear description, and proper schema usage. The stub implementation is appropriate for establishing the interface.docs/blog/external-links.mdx (1)
9-20: Updated external links are well-organized.The card structure and links are clear and properly formatted.
libs/plugins/src/codecall/tools/search.schema.ts (1)
4-11: Placeholder schemas are appropriate for scaffolding.The empty input schema and passthrough output schema establish the type structure for progressive implementation. Ensure these are populated with actual schema definitions before the search tool is fully implemented.
libs/plugins/src/codecall/codecall.types.ts (1)
1-141: LGTM! Comprehensive type definitions for CodeCall plugin.The type system is well-structured with clear documentation. The global module augmentation for
ExtendFrontMcpToolMetadatacorrectly extends the FrontMCP tool metadata framework, and the meta-tool contracts (search, describe, execute) provide a solid foundation for the plugin's API surface.apps/demo/src/apps/calculator/tools/abs.tool.ts (1)
7-12: Public API simplification looks good.The change from returning
{ result: number }to a primitivenumbersimplifies the tool's output contract and is consistent with similar updates across other calculator tools (add, subtract, ceil, floor, exp, max, min, modulo, multiply, pow, round, sqrt).apps/demo/src/apps/calculator/tools/subtract.tool.ts (1)
7-12: Consistent API simplification.The output schema change from object to primitive aligns with the calculator tool refactor pattern seen across the codebase.
libs/plugins/src/codecall/codecall.symbol.ts (1)
1-65: LGTM! Clean public API surface for CodeCall.The symbol-based DI token pattern is appropriate, and the interfaces for AST validation and VM environment provide clear contracts for the plugin's runtime components.
apps/demo/src/apps/calculator/tools/sqrt.tool.ts (1)
7-12: Consistent with calculator tool API refactor.The change to return a primitive number instead of a wrapped object aligns with the broader simplification across all calculator tools.
docs/blog/11-2025/codecall-plugin.mdx (1)
1-667: Excellent user-facing documentation.This blog post effectively introduces the CodeCall plugin with:
- Clear motivation (tool overload problem)
- Comparison to Anthropic's "Code execution with MCP" pattern
- Practical examples and deployment guidance
- Security and privacy explanations
- Multi-app server support details
The content is well-structured and provides a good entry point for users considering the CodeCall plugin.
apps/demo/src/apps/calculator/tools/multiply.tool.ts (1)
1-14: LGTM! Output schema normalization is consistent.The migration from
{ result: number }to a primitivenumberoutput is correct and aligns with the broader refactor across all calculator tools in this PR.libs/plugins/src/codecall/providers/codecall-ast-validator.provider.ts (1)
22-74: LGTM! Comprehensive AST validation implementation.The validation logic correctly:
- Parses JavaScript with Acorn and captures parse errors with location info
- Walks the AST to detect forbidden identifiers from both
disabledBuiltinsanddisabledGlobals- Properly classifies issues based on the source list
- Covers all loop constructs when
allowLoopsis falseapps/demo/src/apps/calculator/tools/round.tool.ts (1)
11-21: LGTM! Consistent output schema normalization.The migration to primitive
numberoutput aligns with the broader calculator tools refactor. The rounding logic remains correct.apps/demo/src/apps/calculator/tools/pow.tool.ts (1)
1-14: LGTM! Consistent with calculator tools refactor.The changes align with the output schema normalization across all calculator tools. The power calculation logic is correct.
apps/demo/src/apps/employee-time/tools/list-employees.tool.ts (2)
14-27: Excellent! Concrete output schema replaces permissive placeholder.The new schema properly validates the output structure with appropriate constraints (total ≥ 0, limit 1-200, offset ≥ 0) and clearly documents each field. This is a significant improvement over the previous
{} as anyplaceholder.
31-52: LGTM! Flattened output structure with proper site enrichment.The execution logic correctly:
- Fetches paginated employee list from the directory provider
- Enriches each employee with their associated sites via
Promise.all- Returns a flat structure matching the new output schema
docs/snippets/card.jsx (1)
3-101: LGTM! Well-structured BlogCard with dark mode support.The refactored component:
- Adds
imgDarkprop for theme-specific images- Uses proper semantic HTML with accessible anchor wrapper
- Implements responsive flex layout (column on mobile, row on desktop)
- Includes hover states and proper styling
- Maintains accessibility with alt attributes
apps/demo/src/apps/employee-time/tools/admin-add-entry.tool.ts (1)
12-20: Entry tool logic and schema look consistentThis tool’s behavior (conflict check, setting open session, directory update, flat
{ employeeId, startedAt }output) is consistent with the exit tools and looks correct. As withadmin-add-exit, theadminflag is enforced only by the input schema; add a runtime guard if you expect any unvalidated callers.Also applies to: 23-39
libs/plugins/src/codecall/providers/codecall-ast-validator.provider.ts
Outdated
Show resolved
Hide resolved
# Conflicts: # apps/demo/src/apps/employee-time/plugins/site-authorization.plugin.ts # apps/demo/src/apps/expenses/index.ts # apps/demo/src/apps/expenses/tools/add.tool.ts # apps/demo/src/apps/expenses/tools/create-expense.tool.ts # apps/demo/src/main.ts # docs/blog/11-2025/introducing-frontmcp.mdx # docs/blog/11-2025/mcp-run-out-of-socket.mdx # docs/blog/external-links.mdx # docs/blog/index.mdx # docs/docs.json # docs/snippets/card.jsx # libs/plugins/package.json # libs/plugins/src/cache/cache.plugin.ts # libs/sdk/src/common/decorators/tool.decorator.ts # libs/sdk/src/common/metadata/tool.metadata.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
docs/blog/index.mdx(1 hunks)libs/plugins/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/plugins/package.json
🧰 Additional context used
📓 Path-based instructions (1)
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/blog/index.mdx
⏰ 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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.claude/settings.local.json(1 hunks)libs/adapters/eslint.config.mjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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/adapters/eslint.config.mjs
⏰ 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 (2)
.claude/settings.local.json (1)
12-12: Verify thatnpx nx run:*permission is necessary and justified.The new permission
Bash(npx nx run:*)is significantly broader than existing entries, which specify particular operations (build, test, lint). Thenx runcommand is a generic task runner that can execute any configured nx target, which may pose a security risk.Before approving, clarify:
- What specific nx task(s) does the CodeCall plugin need to execute?
- Can the permission be narrowed to specific targets (e.g.,
Bash(npx nx run:codecall:*))?- Is the broad scope justified by operational requirements?
libs/adapters/eslint.config.mjs (1)
31-33: Verify whether test directories should be completely ignored.Ignoring
**/__tests__means ESLint won't run on test files at all. While ignoringout-tsc(compiled output) is standard, completely skipping linting for tests might not be intentional.Typically, test files benefit from linting for code quality, with different rules configured via overrides if needed rather than complete exclusion.
Please confirm this is the intended behavior. If tests should be linted with different rules, consider using a rules override block instead:
{ files: ['**/__tests__/**/*.ts', '**/*.spec.ts', '**/*.test.ts'], rules: { // test-specific rule overrides }, }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
libs/plugins/src/codecall/tools/describe.schema.ts (1)
23-24: Consider using more specific types for JSON Schema objects.The
z.any()type forinputSchemaandoutputSchemais very permissive. While this may be necessary given the dynamic nature of JSON Schema objects, consider using a more specific type for better type safety and validation.You could refine this with a more structured type:
- inputSchema: z.any().describe('JSON Schema object describing the tool input parameters'), - outputSchema: z.any().describe('JSON Schema object describing the tool output structure'), + inputSchema: z.record(z.unknown()).describe('JSON Schema object describing the tool input parameters'), + outputSchema: z.record(z.unknown()).describe('JSON Schema object describing the tool output structure'),Or define a more specific JSON Schema structure if the shape is consistent. However, if tool schemas vary significantly, the current
z.any()approach may be acceptable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
libs/plugins/src/codecall/tools/describe.schema.ts(1 hunks)libs/plugins/src/codecall/tools/describe.tool.ts(1 hunks)libs/plugins/src/codecall/tools/search.schema.ts(1 hunks)libs/plugins/src/codecall/tools/search.tool.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/plugins/src/codecall/tools/search.tool.ts
🧰 Additional context used
📓 Path-based instructions (1)
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/plugins/src/codecall/tools/describe.schema.tslibs/plugins/src/codecall/tools/search.schema.tslibs/plugins/src/codecall/tools/describe.tool.ts
🧬 Code graph analysis (1)
libs/plugins/src/codecall/tools/describe.tool.ts (3)
libs/plugins/src/codecall/tools/search.tool.ts (1)
Tool(5-100)libs/plugins/src/codecall/tools/describe.schema.ts (4)
describeToolInputSchema(5-12)describeToolOutputSchema(16-43)DescribeToolInput(14-14)DescribeToolOutput(45-45)libs/sdk/src/scope/scope.instance.ts (1)
tools(140-142)
⏰ 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/plugins/src/codecall/tools/search.schema.ts (1)
39-75: LGTM! Well-designed output schema.The output schema is comprehensive with proper validation constraints:
- Relevance scores properly bounded between 0 and 1
- Warning types cover important edge cases (excluded tools not found, no results, partial results)
- Clear descriptions for each field to guide consumers of this API
The structured warnings approach is particularly good for providing actionable feedback to users.
libs/plugins/src/codecall/tools/describe.tool.ts (3)
1-46: LGTM - Well-structured tool metadata and documentation.The tool decorator configuration and comprehensive description provide clear guidance on when and how to use the describe tool within the CodeCall workflow. The cache settings are appropriate for tool metadata.
47-50: LGTM - Correct class structure and type safety.The class properly extends ToolContext and the execute method signature correctly uses the typed input/output from the schema definitions.
91-94: Return structure is correct.The return statement properly matches the
DescribeToolOutputtype and correctly handles the optionalnotFoundfield by only including it when there are items. This will work correctly once the critical issue on line 68 is resolved.libs/plugins/src/codecall/tools/describe.schema.ts (3)
1-3: LGTM - Correct imports for schema definition.Properly imports zod for schema definition and ToolAnnotationsSchema from the MCP SDK for compatibility with the Model Context Protocol standard.
5-12: LGTM - Well-defined input schema with validation.The input schema properly validates that at least one tool name is provided and includes a clear, grammatically correct description with helpful examples.
16-43: Excellent schema design with comprehensive documentation.The output schema is well-structured with detailed descriptions for each field. The inclusion of
usageExamplewith both description and code is particularly valuable, as it directly supports the codecall workflow by showing developers exactly how to use tools in execute scripts. The optionalnotFoundarray properly handles edge cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (3)
libs/plugins/src/codecall/tools/search.schema.ts (2)
26-57: Ensure CodeCall search documentation matches this public schema (inputs, outputs, and field names)Per the
libs/**guideline about public APIs, please double‑check that the CodeCall plugin docs (e.g.docs/draft/docs/plugins/codecall-plugin.mdx) accurately reflect this schema:filter.appIds,excludeToolNames,topK,relevanceScore(notscore), thewarningsarray shape (types, message, optionalaffectedTools), andtotalAvailableTools. Earlier review feedback found mismatches and missing fields; if those haven’t been addressed yet, it would be good to bring the docs fully in sync with these names and semantics.Also applies to: 61-95
34-39: Prevent emptyfilter.appIdsarrays to avoid unintended “no results” searchesAs noted in the earlier review,
appIdscurrently accepts[], which is truthy and causes the search implementation to filter against an empty set (effectively “search in no apps”) instead of behaving like “all apps” when omitted. This contradicts the description and is easy to hit accidentally.Consider requiring at least one appId:
- appIds: z - .array(z.string()) - .optional() + appIds: z + .array(z.string()) + .min(1) + .optional()This keeps the existing description but blocks the ambiguous empty-array case.
libs/plugins/src/codecall/tools/describe.tool.ts (1)
36-68:codecall:describecurrently reports all tools as not foundWith
const toolExists = false;inside the loop, every requestedtoolNameis added tonotFoundandtoolsis never populated. This contradicts the schema/description and makescodecall:describeeffectively non‑functional even when tools exist in the registry.You’ll want to replace the placeholder with a real registry lookup and push properly shaped entries into
tools. For example (adapt to the actualToolRegistryAPI):- // Placeholder: Check which tools exist - for (const toolName of toolNames) { - // TODO: Check if tool exists in index - const toolExists = false; // Placeholder - - if (!toolExists) { - notFound.push(toolName); - continue; - } - - // TODO: Fetch actual tool definition - // Example structure: - // tools.push({ - // name: toolName, - // appId: 'user', // from tool index - // description: 'List users with pagination', // from tool definition - // inputSchema: { /* JSON Schema */ }, - // outputSchema: { /* JSON Schema */ }, - // annotations: { /* MCP annotations */ }, - // usageExample: { - // description: 'Fetch first 10 users', - // code: `const result = await callTool('${toolName}', { limit: 10, offset: 0 });`, - // }, - // }); - } + for (const toolName of toolNames) { + // Example: check tool registry; adjust to real API for ToolContext/ToolRegistry + const registry = this.scope.tools; + const toolDef = registry.get?.(toolName); + + if (!toolDef) { + notFound.push(toolName); + continue; + } + + // Shape this to match describeToolOutputSchema using data from toolDef + tools.push({ + name: toolName, + appId: toolDef.appId, + description: toolDef.description, + inputSchema: toolDef.inputSchema, + outputSchema: toolDef.outputSchema, + annotations: toolDef.annotations, + usageExample: { + description: `Example of calling "${toolName}" via callTool()`, + code: `const result = await callTool('${toolName}', {/* params */});`, + }, + }); + }Until something like this is in place, the end‑to‑end
codecall:search → codecall:describe → codecall:executeflow described in your docs can’t actually work.
🧹 Nitpick comments (2)
libs/plugins/src/codecall/tools/invoke.schema.ts (2)
52-65: Input schema and inferred type look correct; check no-parameter tool scenariosDefining
invokeToolInputSchemaas a Zod object withtool(string) andinput(record of unknowns) plusInvokeToolInput = z.infer<typeof invokeToolInputSchema>is consistent with the other CodeCall tool schemas and resolves the earlier issues around using a plain object andbaseObjectInputType. One thing to double‑check: if you ever expect tools with truly no parameters, confirm that the higher-level pipeline always supplies at least{}forinput; otherwise, you might want to consider whetherinputshould be optional in this schema.
67-84: Output discriminated union is consistent and type-safe; consider centralizing error codesThe
invokeToolOutputSchemadiscriminated union onstatuswithsuccessanderrorbranches is well-formed, and the errortypeenum matches the error codes documented in the description. This gives a clear, stable contract for SDK consumers. If these error types are (or will be) reused by other CodeCall tools, consider extracting them into a shared enum/schema to avoid string drift across files, but that’s optional.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
.husky/pre-commit(1 hunks)libs/plugins/src/codecall/tools/describe.schema.ts(1 hunks)libs/plugins/src/codecall/tools/describe.tool.ts(1 hunks)libs/plugins/src/codecall/tools/execute.schema.ts(1 hunks)libs/plugins/src/codecall/tools/execute.tool.ts(1 hunks)libs/plugins/src/codecall/tools/invoke.schema.ts(1 hunks)libs/plugins/src/codecall/tools/invoke.tool.ts(1 hunks)libs/plugins/src/codecall/tools/search.schema.ts(1 hunks)libs/plugins/src/codecall/tools/search.tool.ts(1 hunks)package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/plugins/src/codecall/tools/execute.tool.ts
- libs/plugins/src/codecall/tools/invoke.tool.ts
- libs/plugins/src/codecall/tools/execute.schema.ts
- libs/plugins/src/codecall/tools/search.tool.ts
🧰 Additional context used
📓 Path-based instructions (1)
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/plugins/src/codecall/tools/invoke.schema.tslibs/plugins/src/codecall/tools/describe.schema.tslibs/plugins/src/codecall/tools/search.schema.tslibs/plugins/src/codecall/tools/describe.tool.ts
🧬 Code graph analysis (1)
libs/plugins/src/codecall/tools/describe.tool.ts (4)
libs/plugins/src/codecall/tools/invoke.tool.ts (1)
Tool(11-80)libs/plugins/src/codecall/tools/search.tool.ts (1)
Tool(12-87)libs/plugins/src/codecall/tools/describe.schema.ts (5)
describeToolDescription(5-32)describeToolInputSchema(34-41)describeToolOutputSchema(45-72)DescribeToolInput(43-43)DescribeToolOutput(74-74)libs/sdk/src/scope/scope.instance.ts (1)
tools(140-142)
⏰ 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 (2)
package.json (1)
9-13: LGTM! Standard Husky and lint-staged integration.The
preparescript andlint-stagedconfiguration follow the standard pattern for integrating Husky and lint-staged. The configuration appropriately targets common file types for formatting with Prettier.libs/plugins/src/codecall/tools/invoke.schema.ts (1)
4-50: Well-structured tool description aligned with behaviorThe description clearly covers when to use
codecall:invoke, workflow, example payload, error modes, and security, and it matches the schema (status values and overall shape). No changes needed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
package.json (1)
20-20: Critical: Invalid package version — installation will fail.
json-schema-to-zod-v3@1.0.0does not exist on npm. This was flagged in the previous review and remains unresolved. Verify the correct package name and version, or consult the codebase to confirm whether an alternative package (e.g.,json-schema-to-zod,@composio/json-schema-to-zod) should be used instead.
🧹 Nitpick comments (1)
libs/plugins/src/codecall/tools/describe.schema.ts (1)
68-69: Consider stronger typing for schema fields.Both
inputSchemaandoutputSchemausez.any()but are described as "JSON Schema object". Since JSON schemas are always objects (not primitives), consider enforcing at least object structure:- inputSchema: z.any().describe('JSON Schema object describing the tool input parameters'), - outputSchema: z.any().describe('JSON Schema object describing the tool output structure'), + inputSchema: z.record(z.any()).describe('JSON Schema object describing the tool input parameters'), + outputSchema: z.record(z.any()).describe('JSON Schema object describing the tool output structure'),This ensures the fields are objects while remaining flexible for various JSON Schema structures. Alternatively, use
z.object({}).passthrough()if you prefer explicit object validation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
libs/plugins/src/codecall/tools/describe.schema.ts(1 hunks)package.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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/plugins/src/codecall/tools/describe.schema.ts
⏰ 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 (5)
package.json (2)
8-13: Husky/lint-staged integration looks good.The new
preparescript andlint-stagedconfiguration are correctly set up for pre-commit code formatting. The file glob patterns and prettier integration are appropriate for the project's file types.
29-29: New devDependencies are current and properly versioned.@mdx-js/mdx@3.1.1 is the latest version, and husky + lint-staged versions are appropriate for the pre-commit hook integration. The mint version update from 4.2.202 to 4.2.207 is a good patch upgrade.
Also applies to: 47-47, 52-53
libs/plugins/src/codecall/tools/describe.schema.ts (3)
5-32: Excellent documentation and workflow guidance.The description constant is comprehensive, well-structured, and provides clear workflow steps with practical examples. The consistent use of tool naming (e.g.,
codecall:execute,codecall:search) throughout makes it easy to follow.
34-57: Uniqueness validation correctly implemented.The schema now properly enforces unique tool names as described. The
superRefineimplementation correctly detects and reports duplicates with a clear error message. This addresses the previous concern about the description claiming uniqueness without validation.
59-90: Clean type exports and well-structured schema file.The type definitions correctly use
z.inferto derive TypeScript types from the Zod schemas. The file is well-organized with clear exports for descriptions, schemas, and types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/sdk/src/flows/flow.instance.ts (1)
289-291: Type mismatch: returning wrong type fromrunStageGroup.Line 290 returns
res.control?.['output'] ?? res.outcome, which is either an output value or a string (outcome), but the function signature specifiesPromise<StageResult>. This should cause a type error.Apply this diff to return the correct type:
} else if (res.outcome !== 'ok') { - return res.control?.['output'] ?? res.outcome; + return res; }If you need to propagate the output differently, you should still return a proper
StageResultobject:- return res.control?.['output'] ?? res.outcome; + return { outcome: res.outcome, control: res.control };
🧹 Nitpick comments (3)
libs/sdk/src/tool/flows/call-tool.flow.ts (1)
88-99: Owner ID wiring for hook filtering looks good; consider deduping tool lookupThe early lookup and
_toolOwnerIdwrite are consistent withfindTooland safe (no behavior change when the tool is missing). You now scangetTools(true)twice per call though; if this ever becomes heavy, consider extracting a shared helper or caching the active tools array in state for reuse infindTool.libs/sdk/src/common/metadata/hook.metadata.ts (1)
1-3: HookMetadata.owner extension is clean and backward compatibleAdding
owner?: EntryOwnerReftoHookMetadatais a non-breaking way to surface ownership for hooks and lines up with the new owner-aware filtering inHookRegistry. Just ensure any code that manually constructs or clonesHookMetadataalso preserves theownerfield.Also applies to: 17-20, 24-25
libs/sdk/src/flows/flow.instance.ts (1)
139-141: Type assertion bypasses safety; consider validation.The
(input as any)?._toolOwnerIdcast removes type safety for accessing an internal property. If_toolOwnerIdis renamed or not set, this could fail silently or cause runtime issues.Consider one of these improvements:
- Add a type guard or validation:
const toolOwnerId = (input as any)?._toolOwnerId; +if (toolOwnerId !== undefined && typeof toolOwnerId !== 'string') { + throw new Error('Invalid _toolOwnerId type'); +}
- Define a typed interface for inputs with toolOwnerId:
interface ToolOwnerInput { _toolOwnerId?: string; } // Then use type narrowing instead of 'any'Based on coding guidelines (libs/** APIs should be reviewed for correctness).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
libs/sdk/src/app/instances/app.local.instance.ts(3 hunks)libs/sdk/src/common/metadata/hook.metadata.ts(2 hunks)libs/sdk/src/flows/flow.instance.ts(10 hunks)libs/sdk/src/hooks/hook.registry.ts(9 hunks)libs/sdk/src/plugin/plugin.registry.ts(3 hunks)libs/sdk/src/tool/flows/call-tool.flow.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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/flows/call-tool.flow.tslibs/sdk/src/common/metadata/hook.metadata.tslibs/sdk/src/hooks/hook.registry.tslibs/sdk/src/plugin/plugin.registry.tslibs/sdk/src/app/instances/app.local.instance.tslibs/sdk/src/flows/flow.instance.ts
🧬 Code graph analysis (3)
libs/sdk/src/hooks/hook.registry.ts (1)
libs/sdk/src/common/interfaces/internal/registry.interface.ts (1)
HookRegistryInterface(30-58)
libs/sdk/src/plugin/plugin.registry.ts (2)
libs/sdk/src/app/instances/app.local.instance.ts (2)
providers(53-55)plugins(61-63)libs/sdk/src/scope/scope.instance.ts (1)
hooks(124-126)
libs/sdk/src/app/instances/app.local.instance.ts (2)
libs/sdk/src/plugin/plugin.registry.ts (1)
PluginRegistry(17-169)libs/sdk/src/tool/tool.registry.ts (1)
ToolRegistry(29-400)
⏰ 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/app/instances/app.local.instance.ts (1)
1-2: App-level owner propagation into registries looks consistentDefining a single
appOwnerand passing it into bothPluginRegistryandToolRegistry, then awaiting all registries withPromise.all, keeps ownership/provenance consistent and the initialization order intact. No issues from an API or lifecycle perspective.Also applies to: 34-38, 40-41, 46-51, 76-76
libs/sdk/src/hooks/hook.registry.ts (1)
14-17: Owner-aware flow hook retrieval helper looks correct; ensure interface/callers are aligned
getFlowHooksForOwnerbuilds cleanly ongetFlowHooks, preserves existing behavior whenownerIdis omitted, and treats missingmetadata.owneras “global” (applies to all owners), which fits the intended model.Also, the tightened
getPrioritytyping andregisterHooksimplementation still respect the existing contract.One thing to double-check: the
HookRegistryInterfacedefinition undercommon/interfaces/internal/registry.interface.tsshould also exposegetFlowHooksForOwner(...), and any flow/Scope wiring should be updated to call this method where owner-scoped behavior is expected.Also applies to: 21-22, 34-35, 46-47, 66-67, 127-128, 139-147, 150-165, 177-179, 187-188
libs/sdk/src/plugin/plugin.registry.ts (1)
34-45: Owner on plugin hooks likely mismatches tool owner used for flow filteringRight now there’s an inconsistency:
- Plugin tools are created with
owner: { kind: 'plugin', id: rec.metadata.name, ref: token }(viaToolRegistry).- Hooks from the same plugin are registered with
metadata.owner = this.owner, i.e. the owner passed intoPluginRegistry(for app-local plugins that’s the app, not the plugin).Since flows now feed
tool.owner.idintogetFlowHooksForOwner, a tool owned by plugin"my-plugin"will be looked up withownerId = 'my-plugin', but the hooks you’ve just registered haveowner.id = appId, so they won’t be returned (only hooks withownerunset will pass the filter).If the intent is “plugin-scoped hooks for plugin-owned tools” (which is what CodeCall-style tooling seems to need), you probably want hook ownership to line up with the plugin/tool owner instead of the parent app. One concrete option:
- // Pass the owner context to nested plugins (plugins can be owned by apps or other plugins) - const pluginOwner = this.owner || { - kind: 'plugin' as const, - id: rec.metadata.name, - ref: token, - }; - - const plugins = new PluginRegistry(providers, rec.metadata.plugins ?? [], pluginOwner); + // This plugin is the direct owner of its nested plugins, tools, and hooks. + const pluginOwner = { + kind: 'plugin' as const, + id: rec.metadata.name, + ref: token, + }; + + const plugins = new PluginRegistry(providers, rec.metadata.plugins ?? [], pluginOwner); @@ - const tools = new ToolRegistry(providers, rec.metadata.tools ?? [], { - kind: 'plugin', - id: rec.metadata.name, - ref: token, - }); + const tools = new ToolRegistry(providers, rec.metadata.tools ?? [], pluginOwner); @@ - if (hooks.length > 0) { - // Add owner information to each hook before registering - const hooksWithOwner = hooks.map((hook) => ({ - ...hook, - metadata: { - ...hook.metadata, - owner: this.owner, - }, - })); + if (hooks.length > 0) { + // Add owner information to each hook before registering (plugin-scoped) + const hooksWithOwner = hooks.map((hook) => ({ + ...hook, + metadata: { + ...hook.metadata, + owner: pluginOwner, + }, + }));This makes the direct plugin the owner for its nested plugins, tools, and hooks, so
tool.owner.idandhook.metadata.owner.idline up for owner-scoped flow hooks.If you intended an app-scoped ownership model instead, you’ll need to adjust either the
ToolRegistryowner or the value that flows pass intogetFlowHooksForOwnerso they all agree on what “owner” means.Also applies to: 89-97, 151-159
libs/sdk/src/flows/flow.instance.ts (4)
95-96: LGTM: Respond handling correctly implemented.The new
respondcase properly returns the flow output viawriteHttpResponse. The type cast one.outputis acceptable here since FlowControl's output type is generic.
198-225: LGTM: Robust outcome handling.The
runListfunction correctly handles all flow control outcomes and properly returnsStageResultobjects. TheignoreRespondoption appropriately allowsdidstages to ignore respond outcomes while still executing.
244-252: LGTM: Correct error propagation.The throw at line 251 (instead of returning) ensures that
didstages don't execute when the main stage fails or aborts. This maintains proper flow control semantics where cleanup hooks run only on success paths.
143-149: No issues found.getFlowHooksForOwnercorrectly handles undefinedtoolOwnerId.The method signature marks
ownerIdas optional (ownerId?: string), and the implementation explicitly handles the undefined case by returning all hooks (line 156-157) when no owner ID is provided. When an owner ID is passed, it filters to include only owner-scoped or global hooks. The call site correctly passestoolOwnerIdwhich may be undefined, and the method's behavior is appropriate for both cases.
# Conflicts: # .claude/settings.local.json # docs/blog/index.mdx # libs/sdk/src/flows/flow.instance.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/sdk/src/common/interfaces/plugin.interface.ts (1)
4-4: Consider using a type alias instead of an empty interface.Per Biome's lint rule, empty interfaces are equivalent to
{}and type aliases are preferred. While the current form is valid, converting to a type alias would align with TypeScript conventions and improve code clarity.-export interface PluginInterface {} +export type PluginInterface = {};
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/blog/index.mdx(1 hunks)libs/sdk/jest.config.ts(2 hunks)libs/sdk/src/__test-utils__/fixtures/hook.fixtures.ts(4 hunks)libs/sdk/src/__test-utils__/fixtures/plugin.fixtures.ts(1 hunks)libs/sdk/src/__test-utils__/helpers/setup.helpers.ts(1 hunks)libs/sdk/src/__test-utils__/mocks/hook-registry.mock.ts(2 hunks)libs/sdk/src/common/interfaces/plugin.interface.ts(1 hunks)libs/sdk/src/provider/__tests__/provider.registry.test.ts(9 hunks)
✅ Files skipped from review due to trivial changes (2)
- libs/sdk/src/provider/tests/provider.registry.test.ts
- libs/sdk/src/test-utils/fixtures/plugin.fixtures.ts
🧰 Additional context used
📓 Path-based instructions (2)
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/__test-utils__/mocks/hook-registry.mock.tslibs/sdk/jest.config.tslibs/sdk/src/__test-utils__/helpers/setup.helpers.tslibs/sdk/src/__test-utils__/fixtures/hook.fixtures.tslibs/sdk/src/common/interfaces/plugin.interface.ts
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/blog/index.mdx
🧬 Code graph analysis (2)
libs/sdk/src/__test-utils__/mocks/hook-registry.mock.ts (1)
libs/sdk/src/common/metadata/hook.metadata.ts (1)
HookMetadata(17-25)
libs/sdk/src/__test-utils__/fixtures/hook.fixtures.ts (1)
libs/sdk/src/common/metadata/hook.metadata.ts (1)
HookMetadata(17-25)
🪛 Biome (2.1.2)
libs/sdk/src/common/interfaces/plugin.interface.ts
[error] 4-4: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (10)
libs/sdk/src/__test-utils__/helpers/setup.helpers.ts (1)
38-38: LGTM! Trailing comma improves consistency.The trailing comma follows TypeScript best practices and makes future diffs cleaner if parameters are added later. No functional impact.
libs/sdk/jest.config.ts (2)
5-5: Formatting change with no functional impact.This change collapses the swcJestConfig initialization to a single line without altering behavior.
17-17: Formatting change with no functional impact.This change reformats the transformIgnorePatterns array to a single line without altering behavior.
docs/blog/index.mdx (1)
25-38: CodeCall plugin banner image verified—no action required.The image at
docs/assets/banners/codecall-plugin/code-call-plugin.pngexists and is correctly referenced in the BlogCard component. The blog post will render without missing asset issues.libs/sdk/src/__test-utils__/mocks/hook-registry.mock.ts (2)
57-66: LGTM - Formatting improvement.The function signature consolidation to a single line improves readability without affecting behavior or the test utility's contract.
71-73: LGTM - Formatting improvement.The single-line signature is appropriate for this simple helper function and maintains type safety with the
ReturnType<typeof createMockHookRegistry>usage.libs/sdk/src/__test-utils__/fixtures/hook.fixtures.ts (4)
17-27: LGTM - Formatting improvement.The single-line signature is clean and appropriate for this fixture helper with a single parameter.
32-41: LGTM - Formatting improvement.The single-line signature is readable and maintains the default parameter values for both
flowandstage.
46-55: LGTM - Formatting improvement.The single-line signature maintains consistency with the other hook fixture helpers and preserves all default parameter values.
60-69: LGTM - Formatting improvement.The single-line signature completes the consistent formatting pattern across all hook fixture helpers in this file.
# Conflicts: # docs/draft/blog/11-2025/codecall-plugin.mdx # docs/draft/blog/index.mdx # libs/plugins/package.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/draft/blog/index.mdx (1)
55-67: Prior unresolved issue remains: missing image and slug mismatch (lines 55–67).The previous review flagged two unresolved issues in this file that are still present and will cause rendering failures:
- Missing image asset:
../assets/banners/mcp-run-out-of-socket.svg- URL slug mismatch: link is
/blog/11-2025/mcp-run-out-of-socket(singular) but title says "Ran Out of Sockets" (plural)These should be fixed to prevent blocking the site build.
🧹 Nitpick comments (2)
docs/draft/blog/11-2025/codecall-plugin.mdx (2)
48-71: Minor style: reduce repeated word beginnings for readability.Lines 48–51 show three consecutive sentences starting with "You" (bullet list). While acceptable in technical writing, rewording one or two would improve flow:
- You add **another app** ... - You connect an **OpenAPI adapter** ... - You introduce **per-tenant tools** ... + You add **another app** ... + An **OpenAPI adapter** integration inherits 50–100 endpoints as tools. + **Per-tenant tools**, or variants for different products, are introduced.Same pattern appears at lines 158–167 and 554–558.
520-533: Style: fix hyphenation and subject in bullet list.Line 525 should use hyphenated compound adjective:
one-and-done(notone and done).Line 532 (
Still leveraging CodeCall's...) is a dangling participle; consider rewriting as a complete sentence in the bullet list context:- Still leveraging CodeCall's **modes, allowlists, and filters**. + Still leverage CodeCall's **modes, allowlists, and filters**.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/draft/blog/11-2025/codecall-plugin.mdx(1 hunks)docs/draft/blog/index.mdx(1 hunks)libs/plugins/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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/blog/index.mdxdocs/draft/blog/11-2025/codecall-plugin.mdx
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/plugins/package.json
🪛 LanguageTool
docs/draft/blog/11-2025/codecall-plugin.mdx
[style] ~50-~50: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd inherit 50–100 endpoints as tools. - You introduce per-tenant tools, or vari...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~51-~51: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..., or variants for different products. - You start experimenting with plugins an...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~162-~162: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... list_tools or your context window. - You want the model to: - Combine tools int...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~525-~525: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...mal MCP call: - PII plugin(s) - auth - rate limiting - logging/audit This is perfect for: ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~532-~532: It seems that a subject is missing.
Context: ...ency when orchestration isn’t needed. - Still leveraging CodeCall’s **modes, allowlis...
(SENT_START_STILL_VBG)
[style] ~556-~556: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n** still enforces identities/scopes. - Your logging/rate-limit plugins still ru...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ 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 (3)
docs/draft/blog/11-2025/codecall-plugin.mdx (2)
599-620: Clear articulation of relationship to Anthropic pattern.Lines 599–620 do an excellent job connecting CodeCall to Anthropic's code-execution pattern and differentiating the server-side vs. client-side approaches. This framing will help users understand why CodeCall exists and when to use it. Solid pedagogical structure.
244-287: Code example verified and accurate.All API surface details in the CodeCall plugin integration example (lines 244–287) match the actual implementation:
CodeCallPlugin.init()exists via inheritance fromDynamicPlugin- Configuration options (
mode,topK,maxDefinitions,directCalls,vm) matchCodeCallPluginOptions- VM options (
preset,timeoutMs,allowLoops,disabledBuiltins,disabledGlobals,allowConsole) matchCodeCallVmOptions- Direct calls options (
enabled,allowedTools) matchCodeCallDirectCallsOptions- Tool metadata properties (
visibleInListTools,enabledInCodeCall) matchCodeCallToolMetadata- Imports are correctly exported from
@frontmcp/plugins/codecallUsers can follow this example without issues.
docs/draft/blog/index.mdx (1)
25-39: CodeCall blog card image exists; separate unresolved issue at lines 55–67 remains unfixed.The CodeCall blog card image
../assets/banners/codecall-plugin/code-call-plugin.pngis verified to exist.However, the unresolved issue from the previous review persists: the MCP blog card at lines 55–67 references a missing image asset
../assets/banners/mcp-run-out-of-socket.svg. While this is outside the scope of the current changes, it will cause rendering failures when that card is rendered. Address this separately before or alongside publishing these blog entries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
libs/plugins/src/codecall/providers/codecall-ast-validator.provider.ts (1)
45-66: AST-based checks are strict but still bypassable via non‑identifier access patternsThe identifier and loop visitors give you a clear, conservative policy surface (any appearance of a disabled name or loop construct is flagged, regardless of scope), which is good for security. However, note that this approach won’t catch patterns that avoid an
Identifiernamed after the forbidden symbol, e.g. building names from strings or accessing them indirectly.If the threat model includes hostile scripts, consider this validator as one layer rather than the only guard, and combine it with runtime hardening (e.g., freezing or deleting dangerous globals in the VM sandbox) to make bypasses significantly harder.
Also applies to: 74-83
libs/plugins/src/codecall/providers/codecall-vm2.provider.ts (2)
21-25: Clarify whethercodecallContextshould come fromenvorinput(or both)Right now the sandbox’s
codecallContextis built frominput.contextonly, andenv.codecallContextfromCodeCallVmEnvironmentis ignored. If upstream callers ever rely onenv.codecallContext(e.g., to inject shared or precomputed context), this could cause divergence.Consider either:
- Documenting that this VM runner always uses
input.contextas the single source of truth, or- Merging the two (e.g.
{ ...env.codecallContext, ...(input.context ?? {}) }) if both are expected to participate.
41-47: Consider adding VM‑level hardening in addition to AST validationThe comment notes that runtime hardening (e.g.
vm.freezeof dangerous globals) is “possible” but relies on AST validation to reject accesses. Given that AST checks can’t cover all dynamic access patterns and vm2 has a large attack surface, it’s safer to freeze/remove obviously unsafe globals and builtins in the VM options, aligned withdisabledBuiltins/disabledGlobals.Concretely, you could, for example, freeze or delete entries like
process,require, orFunctionin the sandbox before running the script, so even if AST validation is skipped or bypassed, those capabilities remain unavailable.Also applies to: 49-52
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/plugins/src/codecall/providers/codecall-ast-validator.provider.ts(1 hunks)libs/plugins/src/codecall/providers/codecall-vm2.provider.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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/plugins/src/codecall/providers/codecall-ast-validator.provider.tslibs/plugins/src/codecall/providers/codecall-vm2.provider.ts
🧬 Code graph analysis (2)
libs/plugins/src/codecall/providers/codecall-ast-validator.provider.ts (2)
libs/plugins/src/codecall/providers/codecall-vm2.provider.ts (1)
Provider(10-84)libs/plugins/src/codecall/codecall.symbol.ts (3)
ResolvedCodeCallVmOptions(24-32)CodeCallAstValidationResult(15-18)CodeCallAstValidationIssue(8-13)
libs/plugins/src/codecall/providers/codecall-vm2.provider.ts (3)
libs/plugins/src/codecall/codecall.symbol.ts (2)
ResolvedCodeCallVmOptions(24-32)CodeCallVmEnvironment(46-65)libs/plugins/src/codecall/codecall.types.ts (1)
CodeCallExecuteInput(123-129)libs/plugins/src/codecall/tools/execute.schema.ts (1)
CodeCallExecuteResult(214-220)
⏰ 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 (2)
libs/plugins/src/codecall/providers/codecall-ast-validator.provider.ts (1)
22-43: Parse error handling and result shaping look good
validatecleanly turns acorn parse failures into a singleParseErrorissue and returnsok: false, with optional line/column taken fromerr.loc. This aligns with theCodeCallAstValidationIssue/CodeCallAstValidationResultcontracts and should be easy for callers to consume.libs/plugins/src/codecall/providers/codecall-vm2.provider.ts (1)
18-25: Run pipeline and console/log wiring look consistent and usableThe
runmethod cleanly constructs the sandbox (tools + frozencodecallContext), wiresconsoleonly whenallowConsoleis true, and captures logs into an array that’s conditionally returned. Combined with the structuredok/timeout/runtime_errorresults, this gives callers a clear and debuggable execution surface.Also applies to: 27-37, 58-62
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
docs/draft/blog/11-2025/codecall-plugin.mdx (1)
412-415: ** Missing diagram assets for codecall-multi-app-search images.**This mirrors the critical issue flagged in the previous review. The blog post references two missing image assets at lines 412–415:
../../assets/diagrams/codecall-multi-app-search.dark.png(Line 412)../../assets/diagrams/codecall-multi-app-search.light.png(Line 415)The
docs/assets/diagrams/directory appears to be missing entirely. Either create the directory and add these images, or move the images todocs/assets/banners/codecall-plugin/and update the paths.Can you confirm whether these image assets will be added in a separate commit, or should the paths be updated to point to existing assets?
docs/draft/docs/plugins/codecall-plugin.mdx (1)
95-95: ** Console availability documentation doesn't match VM implementation.**This issue was previously flagged and remains unresolved. The documentation claims
console.log/warn/errorare available whenallowConsole: true(lines 168–170), and the example at lines 296–309 demonstrates console usage. However, theCodeCallVm2Runnerimplementation has console injection commented out, soallowConsoledoesn't actually expose console in the sandbox.This creates a gap between documented and actual behavior. Users following the docs will find console doesn't work.
Choose one approach:
- Implement console injection in
CodeCallVm2Runner: Wiresandbox.consolewhenallowConsole: true, optionally capturing logs to forward tomcpLog/mcpNotify.- Update the docs to clarify that console exposure is not currently implemented, or is optional depending on the preset/implementation.
Please confirm which approach is intended and make corresponding changes.
Also applies to: 168-170, 296-309
🧹 Nitpick comments (3)
docs/draft/blog/11-2025/codecall-plugin.mdx (2)
48-51: Minor: Reduce repeated sentence openings in bullet list.Lines 48–51 have three successive bullets starting with "You" (add, connect, introduce). Consider varying the phrasing for better readability:
- You add **another app** (`user`, `billing`, `ops`, `docs`…). - You connect an **OpenAPI adapter** and inherit 50–100 endpoints as tools. - You introduce **per-tenant tools**, or variants for different products. + Add **another app** (`user`, `billing`, `ops`, `docs`…). + Connect an **OpenAPI adapter** and inherit 50–100 endpoints as tools. + Introduce **per-tenant tools**, or variants for different products.
514-518: Minor: Hyphenate compound adjectiverate-limiting.Line 516 should use a hyphen when the compound modifies the following noun:
- Your **logging/rate-limit plugins** still run. + Your **logging/rate-limiting plugins** still run.docs/draft/docs/plugins/codecall-plugin.mdx (1)
328-332: Minor: Hyphenate compound adjectiverate-limiting.Line 330 should hyphenate the compound adjective modifying "plugins":
- Your **PII plugin** still sees inputs and outputs. - Your **auth plugin** still enforces identities/scopes. - - Your **logging/rate-limit plugins** still run. + - Your **logging/rate-limiting plugins** still run.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
docs/draft/assets/banners/codecall-plugin/code-call-example.pngis excluded by!**/*.pngdocs/draft/assets/banners/codecall-plugin/code-call-options.dark.pngis excluded by!**/*.pngdocs/draft/assets/banners/codecall-plugin/code-call-options.light.pngis excluded by!**/*.pngdocs/draft/assets/banners/codecall-plugin/code-call-plugin.pngis excluded by!**/*.pngdocs/draft/assets/banners/codecall-plugin/early-day-vs-today.dark.pngis excluded by!**/*.pngdocs/draft/assets/banners/codecall-plugin/early-day-vs-today.light.pngis excluded by!**/*.png
📒 Files selected for processing (71)
docs/draft/blog/11-2025/codecall-plugin.mdx(1 hunks)docs/draft/blog/index.mdx(1 hunks)docs/draft/docs.json(2 hunks)docs/draft/docs/plugins/codecall-plugin.mdx(1 hunks)docs/draft/docs/plugins/overview.mdx(1 hunks)docs/draft/docs/v/0.1/adapters/openapi-adapter.mdx(0 hunks)docs/draft/docs/v/0.1/deployment/local-dev-server.mdx(0 hunks)docs/draft/docs/v/0.1/deployment/production-build.mdx(0 hunks)docs/draft/docs/v/0.1/getting-started/installation.mdx(0 hunks)docs/draft/docs/v/0.1/getting-started/quickstart.mdx(0 hunks)docs/draft/docs/v/0.1/getting-started/welcome.mdx(0 hunks)docs/draft/docs/v/0.1/servers/apps.mdx(0 hunks)docs/draft/docs/v/0.1/servers/authentication/local.mdx(0 hunks)docs/draft/docs/v/0.1/servers/authentication/overview.mdx(0 hunks)docs/draft/docs/v/0.1/servers/authentication/remote-proxy.mdx(0 hunks)docs/draft/docs/v/0.1/servers/authentication/remote.mdx(0 hunks)docs/draft/docs/v/0.1/servers/authentication/token.mdx(0 hunks)docs/draft/docs/v/0.1/servers/extensibility/adapters.mdx(0 hunks)docs/draft/docs/v/0.1/servers/extensibility/logging.mdx(0 hunks)docs/draft/docs/v/0.1/servers/extensibility/plugins.mdx(0 hunks)docs/draft/docs/v/0.1/servers/extensibility/providers.mdx(0 hunks)docs/draft/docs/v/0.1/servers/prompts.mdx(0 hunks)docs/draft/docs/v/0.1/servers/resources.mdx(0 hunks)docs/draft/docs/v/0.1/servers/server.mdx(0 hunks)docs/draft/docs/v/0.1/servers/tools.mdx(0 hunks)docs/draft/docs/v/0.2/adapters/openapi-adapter.mdx(0 hunks)docs/draft/docs/v/0.2/deployment/local-dev-server.mdx(0 hunks)docs/draft/docs/v/0.2/deployment/production-build.mdx(0 hunks)docs/draft/docs/v/0.2/getting-started/installation.mdx(0 hunks)docs/draft/docs/v/0.2/getting-started/quickstart.mdx(0 hunks)docs/draft/docs/v/0.2/getting-started/welcome.mdx(0 hunks)docs/draft/docs/v/0.2/guides/add-openapi-adapter.mdx(0 hunks)docs/draft/docs/v/0.2/guides/caching-and-cache-miss.mdx(0 hunks)docs/draft/docs/v/0.2/guides/customize-flow-stages.mdx(0 hunks)docs/draft/docs/v/0.2/servers/apps.mdx(0 hunks)docs/draft/docs/v/0.2/servers/authentication/local.mdx(0 hunks)docs/draft/docs/v/0.2/servers/authentication/overview.mdx(0 hunks)docs/draft/docs/v/0.2/servers/authentication/remote-proxy.mdx(0 hunks)docs/draft/docs/v/0.2/servers/authentication/remote.mdx(0 hunks)docs/draft/docs/v/0.2/servers/authentication/token.mdx(0 hunks)docs/draft/docs/v/0.2/servers/extensibility/adapters.mdx(0 hunks)docs/draft/docs/v/0.2/servers/extensibility/logging.mdx(0 hunks)docs/draft/docs/v/0.2/servers/extensibility/plugins.mdx(0 hunks)docs/draft/docs/v/0.2/servers/extensibility/providers.mdx(0 hunks)docs/draft/docs/v/0.2/servers/prompts.mdx(0 hunks)docs/draft/docs/v/0.2/servers/resources.mdx(0 hunks)docs/draft/docs/v/0.2/servers/server.mdx(0 hunks)docs/draft/docs/v/0.2/servers/tools.mdx(0 hunks)docs/draft/docs/v/0.3/adapters/openapi-adapter.mdx(0 hunks)docs/draft/docs/v/0.3/deployment/local-dev-server.mdx(0 hunks)docs/draft/docs/v/0.3/deployment/production-build.mdx(0 hunks)docs/draft/docs/v/0.3/getting-started/installation.mdx(0 hunks)docs/draft/docs/v/0.3/getting-started/quickstart.mdx(0 hunks)docs/draft/docs/v/0.3/getting-started/welcome.mdx(0 hunks)docs/draft/docs/v/0.3/guides/add-openapi-adapter.mdx(0 hunks)docs/draft/docs/v/0.3/guides/caching-and-cache-miss.mdx(0 hunks)docs/draft/docs/v/0.3/guides/customize-flow-stages.mdx(0 hunks)docs/draft/docs/v/0.3/servers/apps.mdx(0 hunks)docs/draft/docs/v/0.3/servers/authentication/local.mdx(0 hunks)docs/draft/docs/v/0.3/servers/authentication/overview.mdx(0 hunks)docs/draft/docs/v/0.3/servers/authentication/remote-proxy.mdx(0 hunks)docs/draft/docs/v/0.3/servers/authentication/remote.mdx(0 hunks)docs/draft/docs/v/0.3/servers/authentication/token.mdx(0 hunks)docs/draft/docs/v/0.3/servers/extensibility/adapters.mdx(0 hunks)docs/draft/docs/v/0.3/servers/extensibility/logging.mdx(0 hunks)docs/draft/docs/v/0.3/servers/extensibility/plugins.mdx(0 hunks)docs/draft/docs/v/0.3/servers/extensibility/providers.mdx(0 hunks)docs/draft/docs/v/0.3/servers/prompts.mdx(0 hunks)docs/draft/docs/v/0.3/servers/resources.mdx(0 hunks)docs/draft/docs/v/0.3/servers/server.mdx(0 hunks)docs/draft/docs/v/0.3/servers/tools.mdx(0 hunks)
💤 Files with no reviewable changes (66)
- docs/draft/docs/v/0.3/servers/resources.mdx
- docs/draft/docs/v/0.2/getting-started/quickstart.mdx
- docs/draft/docs/v/0.2/adapters/openapi-adapter.mdx
- docs/draft/docs/v/0.3/servers/authentication/overview.mdx
- docs/draft/docs/v/0.2/servers/authentication/remote-proxy.mdx
- docs/draft/docs/v/0.3/servers/authentication/local.mdx
- docs/draft/docs/v/0.3/adapters/openapi-adapter.mdx
- docs/draft/docs/v/0.1/servers/authentication/overview.mdx
- docs/draft/docs/v/0.2/deployment/production-build.mdx
- docs/draft/docs/v/0.1/servers/authentication/remote.mdx
- docs/draft/docs/v/0.3/guides/customize-flow-stages.mdx
- docs/draft/docs/v/0.2/servers/authentication/overview.mdx
- docs/draft/docs/v/0.1/servers/resources.mdx
- docs/draft/docs/v/0.3/getting-started/quickstart.mdx
- docs/draft/docs/v/0.1/servers/tools.mdx
- docs/draft/docs/v/0.2/servers/extensibility/logging.mdx
- docs/draft/docs/v/0.1/servers/extensibility/plugins.mdx
- docs/draft/docs/v/0.1/deployment/production-build.mdx
- docs/draft/docs/v/0.2/servers/authentication/remote.mdx
- docs/draft/docs/v/0.2/guides/customize-flow-stages.mdx
- docs/draft/docs/v/0.3/servers/prompts.mdx
- docs/draft/docs/v/0.3/servers/extensibility/providers.mdx
- docs/draft/docs/v/0.3/getting-started/installation.mdx
- docs/draft/docs/v/0.3/servers/authentication/remote.mdx
- docs/draft/docs/v/0.2/servers/extensibility/plugins.mdx
- docs/draft/docs/v/0.3/getting-started/welcome.mdx
- docs/draft/docs/v/0.1/getting-started/quickstart.mdx
- docs/draft/docs/v/0.2/servers/tools.mdx
- docs/draft/docs/v/0.3/servers/authentication/token.mdx
- docs/draft/docs/v/0.3/deployment/production-build.mdx
- docs/draft/docs/v/0.2/getting-started/installation.mdx
- docs/draft/docs/v/0.1/getting-started/welcome.mdx
- docs/draft/docs/v/0.2/servers/apps.mdx
- docs/draft/docs/v/0.1/servers/authentication/local.mdx
- docs/draft/docs/v/0.3/deployment/local-dev-server.mdx
- docs/draft/docs/v/0.2/getting-started/welcome.mdx
- docs/draft/docs/v/0.2/servers/resources.mdx
- docs/draft/docs/v/0.1/servers/authentication/token.mdx
- docs/draft/docs/v/0.3/servers/server.mdx
- docs/draft/docs/v/0.3/servers/authentication/remote-proxy.mdx
- docs/draft/docs/v/0.1/servers/authentication/remote-proxy.mdx
- docs/draft/docs/v/0.2/servers/prompts.mdx
- docs/draft/docs/v/0.3/servers/tools.mdx
- docs/draft/docs/v/0.1/deployment/local-dev-server.mdx
- docs/draft/docs/v/0.3/guides/caching-and-cache-miss.mdx
- docs/draft/docs/v/0.3/guides/add-openapi-adapter.mdx
- docs/draft/docs/v/0.1/servers/extensibility/logging.mdx
- docs/draft/docs/v/0.2/deployment/local-dev-server.mdx
- docs/draft/docs/v/0.2/servers/authentication/local.mdx
- docs/draft/docs/v/0.2/servers/authentication/token.mdx
- docs/draft/docs/v/0.1/servers/prompts.mdx
- docs/draft/docs/v/0.3/servers/extensibility/plugins.mdx
- docs/draft/docs/v/0.3/servers/extensibility/logging.mdx
- docs/draft/docs/v/0.2/servers/extensibility/adapters.mdx
- docs/draft/docs/v/0.1/getting-started/installation.mdx
- docs/draft/docs/v/0.2/servers/extensibility/providers.mdx
- docs/draft/docs/v/0.1/adapters/openapi-adapter.mdx
- docs/draft/docs/v/0.2/guides/add-openapi-adapter.mdx
- docs/draft/docs/v/0.2/servers/server.mdx
- docs/draft/docs/v/0.1/servers/extensibility/adapters.mdx
- docs/draft/docs/v/0.3/servers/apps.mdx
- docs/draft/docs/v/0.3/servers/extensibility/adapters.mdx
- docs/draft/docs/v/0.1/servers/extensibility/providers.mdx
- docs/draft/docs/v/0.1/servers/server.mdx
- docs/draft/docs/v/0.1/servers/apps.mdx
- docs/draft/docs/v/0.2/guides/caching-and-cache-miss.mdx
✅ Files skipped from review due to trivial changes (1)
- docs/draft/docs/plugins/overview.mdx
🧰 Additional context used
📓 Path-based instructions (2)
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/blog/index.mdxdocs/draft/docs.jsondocs/draft/docs/plugins/codecall-plugin.mdxdocs/draft/blog/11-2025/codecall-plugin.mdx
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/plugins/codecall-plugin.mdx
🪛 LanguageTool
docs/draft/docs/plugins/codecall-plugin.mdx
[uncategorized] ~330-~330: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...scrubbing/masking) - Auth plugins - Rate limiting - Audit logging - CodeCall sees the ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
docs/draft/blog/11-2025/codecall-plugin.mdx
[style] ~50-~50: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd inherit 50–100 endpoints as tools. - You introduce per-tenant tools, or vari...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~51-~51: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..., or variants for different products. - You start experimenting with plugins an...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~162-~162: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... list_tools or your context window. - You want the model to: - Combine tools int...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~516-~516: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...mal MCP call: - PII plugin(s) - auth - rate limiting - logging/audit This is perfect for: ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~523-~523: It seems that a subject is missing.
Context: ...ency when orchestration isn’t needed. - Still leveraging CodeCall’s **modes, allowlis...
(SENT_START_STILL_VBG)
[style] ~547-~547: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n** still enforces identities/scopes. - Your logging/rate-limit plugins still ru...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (3)
docs/draft/docs.json (1)
122-122: Navigation entries are well-structured and consistent with existing patterns.The plugin and blog navigation updates are straightforward and follow the established structure. The ordering (overview → cache → codecall) is logical.
Also applies to: 151-151
docs/draft/blog/index.mdx (1)
25-37: BlogCard entry is well-structured and properly positioned.The new CodeCall plugin blog card is correctly formatted, includes all required metadata, and the image path aligns with the asset referenced in the corresponding blog post. Chronological ordering (Nov 22 first) is appropriate.
docs/draft/docs/plugins/codecall-plugin.mdx (1)
1-50: Documentation structure is comprehensive and well-organized.The plugin documentation provides excellent coverage of CodeCall's purpose, configuration, and usage patterns. The "Why Use" section, step-by-step "How It Works," configuration reference with sensible defaults, and extensive examples make this accessible to developers. The troubleshooting section and best practices accordions add practical value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/draft/docs/plugins/codecall-plugin.mdx (1)
282-321: Clarify the availability and behavior ofmcpLogandmcpNotifyfunctions.Lines 314–316 show
mcpLogandmcpNotifyin an example, but the docs don't clearly explain:
- When these functions are available (are they separate from
allowConsole? Always available? Optional?)- What they do and where their output goes (compared to
console.log)- Whether they require any special configuration
The reader must infer from context that these are distinct from
console.logand seem to be independent features. Add a brief explanation before the example, or link to where they're documented.Additionally, line 296 says "Enable
allowConsoleto let models add custom logging" but doesn't state whereconsole.logoutput goes (server logs? returned in response?).Add brief definitions before lines 298–309:
When `allowConsole: true`, execution plans can use: - **`console.log/warn/error`** – Direct console output (appears in server logs) - **`mcpLog(level, message, context)`** – Structured logging - **`mcpNotify(event, data)`** – Emit lifecycle notifications For example:
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/draft/docs/plugins/codecall-plugin.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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/plugins/codecall-plugin.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/plugins/codecall-plugin.mdx
🪛 LanguageTool
docs/draft/docs/plugins/codecall-plugin.mdx
[uncategorized] ~330-~330: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...scrubbing/masking) - Auth plugins - Rate limiting - Audit logging - CodeCall sees the ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/ast-guard/src/rules/unknown-global.rule.ts (1)
32-39: Optional: Literal values won't match Identifier nodes.The entries
'null','true', and'false'are represented asLiteralnodes in the acorn AST, notIdentifiernodes, so they will never match during validation. Including them is harmless but unnecessary. The comment acknowledges this, so this is purely a code cleanliness suggestion.If you prefer to keep only active entries:
const STANDARD_SAFE_GLOBALS = new Set([ - // Literals (handled by parser, but included for completeness) 'undefined', - 'null', - 'true', - 'false', 'Infinity', 'NaN',
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/draft/docs/guides/ast-guard.mdx(2 hunks)libs/ast-guard/src/rules/unknown-global.rule.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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/ast-guard/src/rules/unknown-global.rule.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/guides/ast-guard.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/guides/ast-guard.mdx
⏰ 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 (1)
docs/draft/docs/guides/ast-guard.mdx (1)
206-229: Past review comment resolved: Documentation for UnknownGlobalRule is comprehensive.This section successfully documents the new public API as requested in the previous review. The documentation includes:
- Clear purpose and whitelist-based approach explanation
- Configuration options table with defaults and descriptions
- Usage example showing rule instantiation
- Important note about the flat symbol table limitation with context
The documentation quality matches existing sections and provides actionable guidance for users. Based on coding guidelines for
libs/**anddocs/draft/docs/**.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/draft/docs/guides/ast-guard.mdx (1)
380-380: Fix apparent typo: "errorgreat" should be "error. Great".Line 380 has: "
stopOnFirstErrorhalts validation as soon as a rule reports an errorgreat for latency-sensitive flows." This appears to be a typo where two words were accidentally joined. Apply this fix:-- `stopOnFirstError` halts validation as soon as a rule reports an errorgreat for latency-sensitive flows. +- `stopOnFirstError` halts validation as soon as a rule reports an error—great for latency-sensitive flows.Also applies to: 380-380
♻️ Duplicate comments (1)
docs/draft/docs/plugins/codecall-plugin.mdx (1)
1069-1069: Fix inconsistent function name in JSON example.Line 1069 shows
await call('users:getById'...)but should beawait callTool(...)to match the documented API (line 1053) and other examples throughout the file (lines 242, 248, 467, 471, 629-631, etc.).Apply this diff:
- "script": "const user = await call('users:getById', { id: '123' });", + "script": "const user = await callTool('users:getById', { id: '123' });",
🧹 Nitpick comments (6)
docs/draft/docs/guides/enclave.mdx (1)
184-184: Simplify wordy phrasing.Line 184 could be more concise. Consider: "Enclave returns a structured result indicating success or errors, along with execution stats" instead of the current phrasing.
docs/draft/docs/guides/ast-guard.mdx (1)
4-4: Consider simpler language than "Bank-grade" for technical documentation.The description uses "Bank-grade" as marketing language. For technical documentation, consider a more neutral phrasing like "Extensible JavaScript AST validation" or "Enterprise-grade JavaScript AST validation" if you want to emphasize production-readiness.
Also applies to: 4-4
docs/draft/blog/11-2025/codecall-plugin.mdx (4)
48-51: Reduce repetitive sentence openings in the tool list problem section.Four successive bullet points start with "You," making the prose feel monotonous. Consider varying the sentence structure for better readability.
- You add **another app** (`user`, `billing`, `ops`, `docs`…). - You connect an **OpenAPI adapter** and inherit 50–100 endpoints as tools. - You introduce **per-tenant tools**, or variants for different products. - You start experimenting with **plugins** and cross-cutting helpers. + You add **another app** (`user`, `billing`, `ops`, `docs`…). + Then **OpenAPI adapters** inherit 50–100 endpoints as tools. + **Per-tenant variants** emerge for different products. + Experiments with **plugins** and cross-cutting helpers follow.
160-167: Refactor repetitive "You" sentence openings in the use-case section.Three successive bullet points start with "You." Vary the structure for improved rhythm and clarity.
- - You have **many tools** (inline + OpenAPI, multiple apps). - - You don't want to blow up `list_tools` or your context window. - - You want the model to: + - You have **many tools** (inline + OpenAPI, multiple apps). + - Keeping `list_tools` lightweight is a priority. + - The model should be able to:
522-527: Fix grammatical issue: supply missing subject in line 526.The phrase "Still leveraging CodeCall's..." is a dangling modifier. Add a subject to form a complete sentence.
- This is perfect for: - - - Simple "one-and-done" actions. - - Reducing complexity and latency when orchestration isn't needed. - - Still leveraging CodeCall's **modes, allowlists, and filters**. + This is perfect for: + + - Simple "one-and-done" actions. + - Reducing complexity and latency when orchestration isn't needed. + - You can still leverage CodeCall's **modes, allowlists, and filters**.
547-551: Reduce repetitive "Your" sentence openings in the PII/privacy section.Three successive bullet points start with "Your," creating tonal repetition. Vary the sentence structure.
- - Your **PII plugin** still sees inputs and outputs. - - Your **auth plugin** still enforces identities/scopes. - - Your **logging/rate-limit plugins** still run. + - The **PII plugin** still sees inputs and outputs. + - The **auth plugin** still enforces identities/scopes. + - **Logging and rate-limiting plugins** still run.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/draft/blog/11-2025/codecall-plugin.mdx(1 hunks)docs/draft/docs/guides/ast-guard.mdx(4 hunks)docs/draft/docs/guides/enclave.mdx(1 hunks)docs/draft/docs/plugins/codecall-plugin.mdx(1 hunks)docs/draft/docs/plugins/overview.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/draft/docs/plugins/overview.mdx
🧰 Additional context used
📓 Path-based instructions (2)
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/guides/enclave.mdxdocs/draft/docs/plugins/codecall-plugin.mdxdocs/draft/docs/guides/ast-guard.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/guides/enclave.mdxdocs/draft/docs/plugins/codecall-plugin.mdxdocs/draft/docs/guides/ast-guard.mdxdocs/draft/blog/11-2025/codecall-plugin.mdx
🪛 LanguageTool
docs/draft/docs/guides/enclave.mdx
[style] ~184-~184: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ...ts Enclave returns a structured result with success/error status and execution stats: ```t...
(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
docs/draft/docs/plugins/codecall-plugin.mdx
[uncategorized] ~333-~333: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...scrubbing/masking) - Auth plugins - Rate limiting - Audit logging - CodeCall sees the ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
docs/draft/blog/11-2025/codecall-plugin.mdx
[style] ~50-~50: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd inherit 50–100 endpoints as tools. - You introduce per-tenant tools, or vari...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~51-~51: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..., or variants for different products. - You start experimenting with plugins an...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~162-~162: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... list_tools or your context window. - You want the model to: - Combine tools int...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~519-~519: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...mal MCP call: - PII plugin(s) - auth - rate limiting - logging/audit This is perfect for: ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~526-~526: It seems that a subject is missing.
Context: ...ency when orchestration isn’t needed. - Still leveraging CodeCall’s **modes, allowlis...
(SENT_START_STILL_VBG)
[style] ~550-~550: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n** still enforces identities/scopes. - Your logging/rate-limit plugins still ru...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ 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 (8)
docs/draft/docs/guides/enclave.mdx (1)
69-86: Code example uses callTool from script context—verify alignment with globals.The quick-start example (lines 69-86) demonstrates
callToolbeing invoked in user code (line 70:await callTool('users:list', ...)), and configures it viatoolHandler. Confirm that the transformed/runtime code makescallToolavailable in the global scope (either as a direct global or via the wrapper function__safe_callToolpost-transformation) so the example runs as documented.docs/draft/docs/guides/ast-guard.mdx (3)
30-30: Verify the "zero native dependencies" claim.Line 30 claims the package has "zero native dependencies." Confirm this is accurate by checking the actual
ast-guardpackage.json in the codebase or npm registry—this is an important selling point and should be verified to avoid reader disappointment or support burden.Also applies to: 30-30
81-96: Document whether preset customization options are optional and have defaults.The AgentScript preset customization (lines 81-96) shows
allowedGlobals,allowArrowFunctions, andallowedLoops. Confirm and document whether these are optional properties with sensible defaults (as suggested by the comment on line 85: "default: true"), and clarify what the defaults are if not provided. This helps readers understand whether they can callcreateAgentScriptPreset()with no arguments versus needing to provide some options.
335-358: Excellent documentation of UnknownGlobalRule with clear limitations.The whitelist-based approach for UnknownGlobalRule (lines 335-358) is well-documented, including the important caveat about flat symbol table behavior (lines 356-358). This is a good example of surfacing implementation trade-offs and security assumptions to users. No changes needed here.
docs/draft/docs/plugins/codecall-plugin.mdx (1)
1-1458: Overall documentation quality is excellent—well-structured, comprehensive, and user-centric.The documentation effectively transforms the CodeCall plugin from a narrative guide into a component-driven reference with clear configuration surfaces, practical examples, and security guidance. The use of MDX components (CardGroup, Steps, Accordion, etc.) enhances readability and scannability.
Strengths:
- Clear separation of concerns (Plugin-Level vs. Tool-Level configuration)
- Comprehensive AgentScript language reference with allowed/blocked constructs clearly delineated
- Practical multi-app orchestration examples and real-world patterns
- Strong security positioning emphasizing Enclave's defense-in-depth model
- Troubleshooting section addresses common pain points
After fixing the three function name inconsistencies above, this documentation will provide excellent guidance for developers integrating CodeCall.
docs/draft/blog/11-2025/codecall-plugin.mdx (3)
1-150: Overall content structure and technical accuracy look solid.The blog post provides a comprehensive introduction to CodeCall with well-motivated problem framing, clear architectural explanations, and practical configuration examples. The technical content aligns well with the PR objectives, and references to Anthropic's patterns and related docs are accurate. The progressive disclosure (TL;DR → motivation → architecture → use cases → integration) makes it accessible.
620-658: Internal documentation links are properly formatted and traceable.The cross-references to CodeCall docs, guides, and quickstart are consistent with the project's documentation structure and will help readers navigate to more detailed information. Ensure these pages exist in the published docs during site deployment.
Verify that the following internal documentation pages exist in the published site before merging:
/docs/plugins/codecall-plugin(line 631)/docs/guides/ast-guard(line 138)/docs/getting-started/quickstart(line 642)
9-13: All referenced image assets are present and correctly linked.The original review comment flagged missing image assets, but verification confirms all 6 referenced images exist at
docs/draft/assets/banners/codecall-plugin/and are correctly accessible via the relative paths (../../assets/banners/codecall-plugin/) in the blog post. No action required.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (5)
libs/ast-guard/src/rules/no-call-target-assignment.rule.ts (1)
237-273: Consider type-safe error type parameter.The
reportErrormethod is well-implemented with specific messages and structured data. However, thetypeparameter is typed asstring, which allows any value. Consider using a union type for better type safety:+type ErrorType = + | 'assignment' + | 'declaration' + | 'function-declaration' + | 'function-expression' + | 'class-declaration' + | 'destructuring' + | 'destructuring-rest' + | 'parameter' + | 'catch-parameter' + | 'import'; + -private reportError(node: acorn.Node & { name: string }, type: string, context: ValidationContext): void { +private reportError(node: acorn.Node & { name: string }, type: ErrorType, context: ValidationContext): void {This prevents typos and ensures all call sites use valid error types.
libs/ast-guard/src/__tests__/isolation-breakout.spec.ts (2)
100-127: Replaceconsole.warnwith explicit test expectations.Tests use
console.warnto document potential security gaps when debugger statements aren't blocked. This approach has limitations:
- Warnings can be overlooked in test output
- Tests pass even when gaps exist
- No programmatic tracking of whether gaps are resolved
Consider one of these alternatives:
- If debugger blocking is not yet implemented, mark these tests with
.skip()and open an issue- If debugger statements are intentionally allowed in STRICT mode, document this as accepted risk with
expect(result.valid).toBe(true)and a comment explaining the rationale- Use `` pattern to verify the current implementation status
Example for explicit documentation of accepted behavior:
it('should block debugger statements in STRICT mode', async () => { const code = ` const secret = 'sensitive data'; debugger; return secret; `; - // Note: debugger statements may be allowed depending on preset - // This test documents expected behavior const result = await strictValidator.validate(code); - // If debugger is not blocked, this is a gap to address - if (result.valid) { - console.warn('SECURITY GAP: debugger statements not blocked in STRICT mode'); - } + // ACCEPTED RISK: debugger statements currently allowed in STRICT mode for debugging + // TODO: Consider blocking in production environments + expect(result.valid).toBe(true); });
130-143: Skip or remove placeholder test until Record syntax is available.This test validates a trivial placeholder object rather than actual Record/Tuple syntax. It provides no security verification and will always pass.
Apply this diff to skip the test until the proposal is implemented:
- it('should be prepared to block Record prototype access', async () => { + it.skip('should be prepared to block Record prototype access', async () => { - // When Record becomes available: - // const rec = #{ a: 1 }; - // rec.__proto__.constructor.constructor - const code = ` - // Placeholder test - Record syntax not yet available - const placeholder = { type: 'Record', note: 'Future attack surface' }; - `; - const result = await strictValidator.validate(code); - expect(result.valid).toBe(true); // Placeholder passes + // TODO: Implement when Record syntax becomes available (Stage 2+ proposal) + // Test should validate: + // const rec = #{ a: 1 }; + // rec.__proto__.constructor.constructor });libs/ast-guard/src/__tests__/unicode-security.spec.ts (2)
18-492: Comprehensive UnicodeSecurityRule coverage; consider minor test ergonomics tweaksThis suite does a thorough job exercising homoglyphs (Cyrillic/Greek/fullwidth), bidi/Trojan Source, zero-width, invisible characters, strings/templates/comments toggles, configuration flags, real-world spoofing, and parser-level blocking. A couple of optional cleanups:
- You repeatedly construct
new JSAstValidator([new UnicodeSecurityRule(opts)])in many tests. A small local helper would reduce duplication and keep option wiring consistent:+const createValidator = (options?: any) => + new JSAstValidator([new UnicodeSecurityRule(options)]); @@ - const validator = new JSAstValidator([new UnicodeSecurityRule()]); + const validator = createValidator();
- A few expectations assert on
messagesubstrings (e.g., the homoglyph'a'message, “Trojan Source”, “hide malicious code”). If the human-facing copy isn’t meant to be a stable API surface, you might rely only oncodeand structured fields to avoid brittle test failures on message rewording.- For “should not check string literals/template literals by default” and “should allow legitimate unicode in strings by default”, you currently only assert
valid. Ifvalidever decouples from “no issues”, an extraexpect(result.issues).toHaveLength(0)would make the intent explicit.
349-385: Config options section is good; consider adding explicit blockBidi coverageThe configuration tests nicely cover disabling zero-width, homoglyph, invisible detection and whitelisting characters. To round this out, you could add a small test for
blockBidi: falseto document that bidi attacks can be explicitly allowed even whencheckCommentsis true, e.g.:describe('Configuration Options', () => { @@ it('should allow whitelisting specific characters', async () => { const validator = new JSAstValidator([ new UnicodeSecurityRule({ allowedCharacters: ['\u200C'] }), // Whitelist ZWNJ ]); const code = 'const x\u200Cy = 1;'; // ZWNJ is whitelisted const result = await validator.validate(code); expect(result.valid).toBe(true); }); + + it('should allow disabling bidi detection', async () => { + const validator = new JSAstValidator([ + new UnicodeSecurityRule({ blockBidi: false, checkComments: true }), + ]); + const code = 'const x = 1; // Comment with \u202E bidi'; + const result = await validator.validate(code); + + expect(result.valid).toBe(true); + }); });This would make the behavior of
blockBidias explicit as the other flags.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
libs/ast-guard/src/__tests__/isolation-breakout.spec.ts(1 hunks)libs/ast-guard/src/__tests__/no-global-access-hardening.spec.ts(1 hunks)libs/ast-guard/src/__tests__/unicode-security.spec.ts(1 hunks)libs/ast-guard/src/presets/strict.preset.ts(4 hunks)libs/ast-guard/src/rules/index.ts(1 hunks)libs/ast-guard/src/rules/no-call-target-assignment.rule.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/ast-guard/src/presets/strict.preset.ts
🧰 Additional context used
📓 Path-based instructions (1)
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/ast-guard/src/rules/no-call-target-assignment.rule.tslibs/ast-guard/src/rules/index.tslibs/ast-guard/src/__tests__/unicode-security.spec.tslibs/ast-guard/src/__tests__/no-global-access-hardening.spec.tslibs/ast-guard/src/__tests__/isolation-breakout.spec.ts
🧬 Code graph analysis (1)
libs/ast-guard/src/__tests__/unicode-security.spec.ts (1)
libs/ast-guard/src/rules/unicode-security.rule.ts (3)
UnicodeSecurityRule(231-464)normalizeHomoglyphs(470-486)isUnicodeSafe(492-511)
⏰ 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 (8)
libs/ast-guard/src/__tests__/no-global-access-hardening.spec.ts (2)
1-28: LGTM: Clean test setup with parameterized testing.The test infrastructure is well-structured with a reusable preset matrix that enables testing both Strict and AgentScript presets against the same attack vectors.
37-108: No issues found. Tests correctly align with rule implementation.All five test cases have been verified against the NoGlobalAccessRule implementation:
NO_GLOBAL_DESTRUCTUREcode is properly emitted for destructuring attacks (tests 1 & 3)NO_GLOBAL_ACCESScode is properly emitted with correct data fields:
accessType: 'computed'for dynamic/optional chaining access (tests 2 & 5)globalandpropertyfields for property access detection (test 4)The test suite accurately validates the rule's security controls.
libs/ast-guard/src/rules/no-call-target-assignment.rule.ts (4)
1-20: LGTM: Well-documented options interface.The imports and interface definition are clean and appropriately documented.
56-68: LGTM: Efficient data structure choice.Using a
SetforprotectedTargetsprovides O(1) lookup performance for identifier checks.
73-127: LGTM: Comprehensive AST node coverage.The rule correctly handles all major declaration forms including assignments, variable declarations, functions (regular, expression, arrow), and classes. The logic appropriately checks parameters for functions and uses helper methods for complex destructuring patterns.
155-232: LGTM: Thorough destructuring and parameter handling.The helper methods comprehensively handle all destructuring patterns:
- Object/array destructuring
- Nested patterns
- Assignment patterns (defaults)
- Rest elements
- Function parameters
The recursive approach correctly handles deeply nested patterns.
libs/ast-guard/src/__tests__/unicode-security.spec.ts (2)
1-17: File header and imports are clear and well-scopedThe high-level comment block and imports from
validatorandunicode-security.ruleare clear and keep the test’s dependency surface tight; no issues here.
494-560: Helper function tests look solid; confirm public API/docs alignmentThe
normalizeHomoglyphsandisUnicodeSafetests cover expected mappings and safety classification well (homoglyph replacement, various attack classes, and legitimate Unicode). If these helpers are intended to be part of the publicast-guardAPI (as their direct import from the rule module suggests), please ensure they’re also reflected in the SDK documentation underdocs/draft/docs/**so external consumers know they exist and how they’re meant to be used.
…all target validation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
libs/ast-guard/src/presets/agentscript.preset.ts (1)
92-174: Excellent documentation for the preset function.The JSDoc provides comprehensive documentation including language description, use cases, example code, and security model explanation. This is helpful for consumers of the public API.
The missing documentation update was already flagged in a previous review.
🧹 Nitpick comments (6)
libs/ast-guard/src/__tests__/static-call-target.spec.ts (2)
127-138: Consider adding message assertion for consistency.Unlike similar tests (lines 55, 69, 83, 97, 110, 124, 152), this test doesn't verify the error message content. For consistency and thoroughness, consider adding:
expect(result.valid).toBe(false); expect(result.issues).toHaveLength(1); expect(result.issues[0].code).toBe('DYNAMIC_CALL_TARGET'); + expect(result.issues[0].message).toContain('Logical expressions');
6-365: Consider reducing test setup duplication.Each test instantiates a new
StaticCallTargetRule()andJSAstValidator([rule]). While this ensures test isolation, you could improve maintainability by extracting helper functions or using setup hooks:Option 1: Helper function
function createValidator(options?: StaticCallTargetOptions) { const rule = new StaticCallTargetRule(options); return new JSAstValidator([rule]); }Option 2: Setup hook for tests without custom options
describe('StaticCallTargetRule', () => { let validator: JSAstValidator; beforeEach(() => { const rule = new StaticCallTargetRule(); validator = new JSAstValidator([rule]); }); // ... tests using shared validator });This would reduce boilerplate while maintaining clarity.
libs/ast-guard/src/__tests__/transforms.spec.ts (1)
6-10: Consider extracting duplicateparseCodehelper to module scope.The
parseCodehelper function is defined identically in bothdescribeblocks (lines 6-10 and 210-214). Extract it to module scope to reduce duplication.import * as acorn from 'acorn'; import { generate } from 'astring'; import { extractLargeStrings, shouldExtract, transformConcatenation, transformTemplateLiterals } from '../transforms'; +const parseCode = (code: string) => + acorn.parse(code, { + ecmaVersion: 'latest', + sourceType: 'module', + }); + describe('String Extraction Transform', () => { - const parseCode = (code: string) => - acorn.parse(code, { - ecmaVersion: 'latest', - sourceType: 'module', - }); - describe('extractLargeStrings', () => {Also applies to: 210-214
libs/ast-guard/src/presets/agentscript.preset.ts (2)
311-319: Inconsistent boolean logic patterns for loop options.The boolean expressions use different patterns:
- Lines 313, 317:
!== false(enables when undefined or true)- Lines 314-316:
|| false(enables only when explicitly true)While intentional per the defaults, consider using a more explicit pattern for clarity:
rules.push( new ForbiddenLoopRule({ - allowFor: options.allowedLoops?.allowFor !== false, // default true (bounded) - allowWhile: options.allowedLoops?.allowWhile || false, // default false (unbounded) - allowDoWhile: options.allowedLoops?.allowDoWhile || false, // default false (unbounded) - allowForIn: options.allowedLoops?.allowForIn || false, // default false (prototype walking) - allowForOf: options.allowedLoops?.allowForOf !== false, // default true (safe iteration) + allowFor: options.allowedLoops?.allowFor ?? true, // default true (bounded) + allowWhile: options.allowedLoops?.allowWhile ?? false, // default false (unbounded) + allowDoWhile: options.allowedLoops?.allowDoWhile ?? false, // default false (unbounded) + allowForIn: options.allowedLoops?.allowForIn ?? false, // default false (prototype walking) + allowForOf: options.allowedLoops?.allowForOf ?? true, // default true (safe iteration) }), );
234-308: Consider whetherconsoleshould be in the dangerous identifiers list.The comprehensive dangerous identifiers list covers many attack vectors, but
consoleis not included. In a sandbox environment,consolemethods could potentially be used for information disclosure or timing attacks. If AgentScript is meant for untrusted agent code, consider addingconsoleto the list.This is optional depending on your security model — if the sandbox provides a safe
consoleimplementation, leaving it out is fine.libs/ast-guard/src/index.ts (1)
98-110: Consider usingexport typefor type-only exports.
StringExtractionConfig,StringExtractionResult,ConcatTransformConfig, andConcatTransformResultare all interfaces and should use explicitexport typesyntax. This enables better tree-shaking and makes intent clearer.// AST Transforms (for pass-by-reference support) export { // String extraction extractLargeStrings, shouldExtract, - StringExtractionConfig, - StringExtractionResult, // Concatenation transformation transformConcatenation, transformTemplateLiterals, - ConcatTransformConfig, - ConcatTransformResult, } from './transforms'; + +export type { + StringExtractionConfig, + StringExtractionResult, + ConcatTransformConfig, + ConcatTransformResult, +} from './transforms';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
libs/ast-guard/src/__tests__/static-call-target.spec.ts(1 hunks)libs/ast-guard/src/__tests__/transforms.spec.ts(1 hunks)libs/ast-guard/src/index.ts(5 hunks)libs/ast-guard/src/presets/agentscript.preset.ts(1 hunks)libs/ast-guard/src/rules/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/ast-guard/src/rules/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
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/ast-guard/src/__tests__/static-call-target.spec.tslibs/ast-guard/src/__tests__/transforms.spec.tslibs/ast-guard/src/presets/agentscript.preset.tslibs/ast-guard/src/index.ts
🧬 Code graph analysis (3)
libs/ast-guard/src/__tests__/static-call-target.spec.ts (1)
libs/ast-guard/src/rules/static-call-target.rule.ts (1)
StaticCallTargetRule(48-217)
libs/ast-guard/src/__tests__/transforms.spec.ts (3)
libs/ast-guard/src/transforms/index.ts (4)
extractLargeStrings(11-11)shouldExtract(12-12)transformConcatenation(19-19)transformTemplateLiterals(20-20)libs/ast-guard/src/transforms/string-extraction.transform.ts (2)
extractLargeStrings(73-154)shouldExtract(163-165)libs/ast-guard/src/transforms/concat.transform.ts (2)
transformConcatenation(65-103)transformTemplateLiterals(118-163)
libs/ast-guard/src/presets/agentscript.preset.ts (8)
libs/ast-guard/src/rules/no-eval.rule.ts (1)
NoEvalRule(13-89)libs/ast-guard/src/rules/reserved-prefix.rule.ts (1)
ReservedPrefixRule(63-352)libs/ast-guard/src/rules/unknown-global.rule.ts (1)
UnknownGlobalRule(89-369)libs/ast-guard/src/rules/no-user-functions.rule.ts (1)
NoUserDefinedFunctionsRule(81-239)libs/ast-guard/src/rules/no-global-access.rule.ts (1)
NoGlobalAccessRule(29-235)libs/ast-guard/src/rules/disallowed-identifier.rule.ts (1)
DisallowedIdentifierRule(25-91)libs/ast-guard/src/rules/static-call-target.rule.ts (1)
StaticCallTargetRule(48-217)libs/ast-guard/src/rules/required-function-call.rule.ts (1)
RequiredFunctionCallRule(31-133)
⏰ 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 (11)
libs/ast-guard/src/__tests__/static-call-target.spec.ts (1)
4-380: Excellent test coverage and structure.The test suite is comprehensive, covering:
- Acceptance criteria for static literals (strings, templates without expressions)
- Rejection of all dynamic argument types (variables, concatenation, ternary, function calls, property access, logical expressions, await)
- Missing argument handling
- Tool name whitelisting with both string and RegExp patterns
- Custom target functions and argument positions
- Member expression calls
- Multiple calls and violations
- Edge cases (empty strings, special characters, nested calls, non-target functions)
- Location information accuracy
The tests are well-organized and clearly demonstrate the expected behavior of StaticCallTargetRule.
libs/ast-guard/src/__tests__/transforms.spec.ts (4)
12-184: LGTM!The
extractLargeStringstest suite provides comprehensive coverage including threshold boundaries, multiple strings, static vs dynamic template literals, non-string literals, and nested structures.
186-206: LGTM!Good boundary testing for the
shouldExtractfunction, including the important multi-byte UTF-8 character case with emojis.
209-297: LGTM!The
transformConcatenationtest suite properly covers simple and chained concatenation, operator specificity (only+), and custom configuration options.
299-345: LGTM!The
transformTemplateLiteralstest suite correctly verifies that only template literals with expressions are transformed, while static templates remain unchanged.libs/ast-guard/src/presets/agentscript.preset.ts (2)
19-90: Well-designed interface with clear documentation.The
AgentScriptOptionsinterface is well-structured with comprehensive JSDoc comments explaining each option's purpose and defaults.
175-177: LGTM!The
createAgentScriptPresetfunction is well-structured, building a comprehensive validation pipeline with sensible defaults and appropriate configurability.Also applies to: 360-360
libs/ast-guard/src/index.ts (4)
10-16: LGTM!New transformer exports are properly organized with clear comments distinguishing the core transformer from the AgentScript-specific transformer.
27-30: LGTM!Transform-related types are properly added to the interface exports block.
53-58: LGTM!New rule classes and their options types are exported following the established pattern.
Also applies to: 70-75
85-97: LGTM!The AgentScript preset and description exports are properly organized with clear section comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (2)
docs/draft/docs/guides/enclave.mdx (1)
266-285: Minor wording improvement on result structure description.Line 268 could be slightly more concise. Consider revising:
-Enclave returns a structured result with success/error status and execution stats: +Enclave returns a structured result with status and execution stats:The
ExecutionResultinterface clearly shows the success/error distinction, so the prefix is somewhat redundant. This is purely stylistic and optional.docs/draft/docs/plugins/codecall-plugin.mdx (1)
354-370: Capitalize "Audit logging" for consistency with other list items.In the PII and Privacy section, the bullet list at line 359–362 starts with capitalized "PII plugins" but "Audit logging" appears lowercase. For consistency with the other capitalized bullets, consider:
- PII plugins (scrubbing/masking) - Auth plugins - Rate limiting - - Audit logging + - Audit LoggingThis is a minor style issue; lowercase is also acceptable if you prefer lowercase for all non-proper nouns.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/draft/docs/guides/ast-guard.mdx(4 hunks)docs/draft/docs/guides/enclave.mdx(1 hunks)docs/draft/docs/plugins/codecall-plugin.mdx(1 hunks)libs/ast-guard/README.md(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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/plugins/codecall-plugin.mdxdocs/draft/docs/guides/enclave.mdxdocs/draft/docs/guides/ast-guard.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/plugins/codecall-plugin.mdxdocs/draft/docs/guides/enclave.mdxdocs/draft/docs/guides/ast-guard.mdx
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/ast-guard/README.md
🪛 LanguageTool
docs/draft/docs/plugins/codecall-plugin.mdx
[uncategorized] ~361-~361: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...scrubbing/masking) - Auth plugins - Rate limiting - Audit logging - CodeCall sees the ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
docs/draft/docs/guides/enclave.mdx
[style] ~268-~268: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ...ts Enclave returns a structured result with success/error status and execution stats: ```t...
(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
🪛 markdownlint-cli2 (0.18.1)
libs/ast-guard/README.md
13-13: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
805-805: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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 (8)
docs/draft/docs/plugins/codecall-plugin.mdx (2)
59-112: Approve Quick Start and Configuration sections.The Quick Start (lines 59–102) clearly introduces the four meta-tools and the configuration sections (lines 115–259) are well-organized with clear field descriptions, defaults, and examples. Tool-level configuration examples (lines 209–244) are particularly helpful in showing different modes and patterns.
464-750: Approve AgentScript Language Reference and Syntax sections.The AgentScript reference (lines 464–750) is comprehensive and well-structured:
- Clear "What you CAN/CANNOT do" card comparison (lines 470–489)
- Syntax quick reference with helpful examples (lines 493–531)
- Allowed globals table with good descriptions (lines 533–551)
- Blocked constructs organized by category with rationale (lines 553–632)
- Reserved identifiers clearly explained (lines 634–649)
- Common patterns section with practical examples (lines 651–724)
The documentation will help users write correct code quickly and understand why constructs are blocked.
docs/draft/docs/guides/ast-guard.mdx (4)
47-115: Approve Quick Start and AgentScript Preset sections.Quick Start (lines 47–70) is concise and immediately usable. The AgentScript preset section (lines 74–114) clearly documents:
- Purpose and use cases (line 76)
- Configuration options with sensible defaults (lines 81–99)
- Option table with descriptions (lines 102–114)
- Practical tip about
requireCallTool(lines 112–114)Users can quickly understand presets and customize behavior. Cross-reference to Enclave is helpful.
166-204: Approve Code Transformation documentation.The code transformation section (lines 166–204) provides:
- Clear before/after example showing actual transformations (lines 173–195)
- Explanation of each transformation (lines 197–202)
- Documentation of reserved prefixes (
__ag_,__safe_)This helps users understand what happens to their code under the hood and why certain identifiers are forbidden.
317-375: Approve custom rules and UnknownGlobalRule documentation.The custom rules section (lines 317–375) demonstrates:
- Practical mix-in of built-in rules (lines 331–349)
- Introduction of
UnknownGlobalRuleas new public API (line 345)- Clear whitelist-based approach explanation (lines 352–365)
- Options table with descriptions (lines 367–371)
- Important caveat about flat symbol table and its implications (lines 373–375)
The note about flat symbol table behavior is particularly valuable for users who may enable user functions in the future.
241-315: Approve validation patterns and platform-level enforcement.Sections on validation in tools (lines 241–275) and platform-level enforcement (lines 277–315) provide:
- Practical tool integration example with error handling (lines 245–275)
- Platform-level plugin pattern for centralized validation (lines 281–315)
- Clear explanation of hook-based approach
- Real options:
maxIssues,stopOnFirstError, rule configuration per requestThese patterns give users multiple integration strategies depending on their architecture.
libs/ast-guard/README.md (2)
841-847: All referenced documentation files are present in the codebase.The verification confirms that all five files referenced in the README table exist at the expected locations in
libs/ast-guard/docs/. The documentation structure is complete and the references are accurate.
118-127: NoGlobalAccessRule verification complete — implementation aligns with documented behavior.The rule exists at
libs/ast-guard/src/rules/no-global-access.rule.ts, is properly exported, and all three documented behaviors are correctly implemented:
- Global object access: Blocks window, globalThis, self, global, and this (lines 50–104)
- Constructor chains: Detects .constructor property access in both dot and bracket notation (lines 111–128)
- Reflect API: Detects and blocks Reflect method calls (lines 169–189)
No issues found.
…NoCallTargetAssignmentRule, and UnicodeSecurityRule
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (1)
docs/draft/docs/guides/ast-guard.mdx (1)
478-480: Consider updating VM2 reference.The warning references VM2, which has been deprecated due to unresolvable security issues. Consider mentioning more current alternatives or noting that VM2 is deprecated.
<Warning> - AST Guard prevents unsafe syntax from entering your sandbox, but it does not execute or sandbox code itself. Pair it with your existing isolation layer (VM2, workers, remote runners, etc.) for complete defense-in-depth. + AST Guard prevents unsafe syntax from entering your sandbox, but it does not execute or sandbox code itself. Pair it with your existing isolation layer (isolated-vm, workers, remote runners, etc.) for complete defense-in-depth. </Warning>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/draft/docs/guides/ast-guard.mdx(5 hunks)docs/draft/docs/plugins/codecall-plugin.mdx(1 hunks)libs/ast-guard/README.md(5 hunks)libs/ast-guard/src/__tests__/isolation-breakout.spec.ts(1 hunks)libs/ast-guard/src/index.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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/ast-guard/README.mdlibs/ast-guard/src/__tests__/isolation-breakout.spec.tslibs/ast-guard/src/index.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/plugins/codecall-plugin.mdxdocs/draft/docs/guides/ast-guard.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/plugins/codecall-plugin.mdxdocs/draft/docs/guides/ast-guard.mdx
🪛 LanguageTool
docs/draft/docs/plugins/codecall-plugin.mdx
[uncategorized] ~361-~361: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...scrubbing/masking) - Auth plugins - Rate limiting - Audit logging - CodeCall sees the ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ 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 (21)
docs/draft/docs/plugins/codecall-plugin.mdx (1)
1-1486: Excellent work addressing all prior feedback—documentation is comprehensive and well-aligned with implementation.All major issues from previous reviews have been resolved:
- Console availability and
allowConsoleconfiguration are clearly documented (lines 54, 163–165, 327, 547–551)- Complete API schemas now provided for
codecall.describeandcodecall.invoke(lines 884–982, 984–1072), matching the prior gap- VM runtime docs clarified: Enclave is now the primary execution model (line 54); no isolated-vm inconsistencies remain
- All code examples consistently use the correct
callTool()function signature throughout (lines 270, 495, 657, 1085–1087, 1175, 1198, etc.)The documentation is well-structured with strong coverage of quick start, configuration (plugin-level + tool-level), advanced patterns, AgentScript language reference, meta-tools API, troubleshooting, and a practical multi-app example. The use of CardGroups, Accordions, ParamFields, and CodeGroups makes the content navigable and discoverable.
Minor observation: Per your coding guidelines, since this file is in
docs/draft/docs/**, ensure any future updates to CodeCall plugin docs (new features, configuration changes, etc.) are made here first, and the automation workflow will sync todocs/docs/**.libs/ast-guard/src/__tests__/isolation-breakout.spec.ts (9)
1-32: LGTM! Comprehensive test suite setup.The file header clearly documents the attack vectors being tested, and the test setup properly initializes the two validators (strict and agentScript) needed for different validation scenarios. The past review comment about removing
secureValidatorhas been addressed.
123-173: LGTM! ES2024+ attack surfaces well covered.The skip test for Record/Tuple is appropriately documented with a TODO for the stage 2 proposal. All active tests (Iterator helpers, Symbol.dispose) have proper assertions and correctly validate that dangerous ES2024+ features are blocked.
175-232: LGTM! V8 internal attack vectors thoroughly tested.The tests correctly validate that V8-specific internals (Error.captureStackTrace, ArrayBuffer.transfer, SharedArrayBuffer) are blocked, with appropriate assertions and clear explanations of the security risks.
234-259: LGTM! VM context escape vectors properly tested.The tests correctly validate that VM context escape attempts via
thisbinding andarguments.calleeare blocked by the strict validator.
261-303: LGTM! Timing attack vectors properly tested and documented.The Date.now() tests now have proper assertions (addressing the past review comment). The tests correctly validate that STRICT mode blocks Date access while STANDARD preset allows it with documented risk rationale. The distinction between presets and their security tradeoffs is well documented.
305-361: LGTM! Constructor chain escapes comprehensively tested.The tests provide thorough coverage of constructor chain escape attempts across all primitive and object types (Object, Array, String, Number, Boolean, Function), with proper assertions validating they are blocked.
363-412: LGTM! Reserved prefix attacks properly validated.The tests correctly use
agentScriptValidator(which includesReservedPrefixRule) instead ofstrictValidator. Comments clearly explain this choice and the special handling of__ag_main. All assertions properly validate that reserved prefix manipulation is blocked.
414-457: LGTM! Prototype manipulation attacks well covered.The tests comprehensively validate that prototype manipulation vectors (setPrototypeOf, proto assignment, prototype pollution) are properly blocked, protecting against prototype chain attacks.
459-497: LGTM! Meta-programming abuse vectors properly tested.The tests correctly validate that meta-programming APIs (Proxy, Reflect, Object.defineProperty, Object.getOwnPropertyDescriptor) are blocked, preventing abuse of JavaScript's reflection capabilities for sandbox escapes.
libs/ast-guard/README.md (2)
801-827: Defense-in-Depth Architecture documentation is clear and well-structured.The 4-layer security architecture is well documented with the text code fence properly specified.
839-847: Documentation links are valid; all referenced files exist.libs/ast-guard/src/index.ts (4)
10-15: Transformer exports are well-organized.Good separation of core transformer and AgentScript-specific transformer exports with proper type export syntax.
44-60: New security rules properly exported.The addition of
NoGlobalAccessRule,ReservedPrefixRule,UnknownGlobalRule,NoUserDefinedFunctionsRule,NoCallTargetAssignmentRule,UnicodeSecurityRule, andStaticCallTargetRuleexpands the public API surface appropriately for the AgentScript security model.
92-98: AgentScript description constants exported for AI agent tooling.These constants (
AGENTSCRIPT_DESCRIPTION,AGENTSCRIPT_DESCRIPTION_SHORT, etc.) provide standardized descriptions for AI agents consuming this API.
100-115: All exports verified and correctly organized.The
./transformsmodule exports match perfectly:
- Functions:
extractLargeStrings,shouldExtract(string-extraction.transform.ts line 163),transformConcatenation,transformTemplateLiterals(concat.transform.ts line 118) all present and correctly re-exported- Types:
StringExtractionConfig,StringExtractionResult,ConcatTransformConfig,ConcatTransformResultall correctly defined and re-exportedThe code changes are accurate with no issues identified.
docs/draft/docs/guides/ast-guard.mdx (5)
47-66: Quick Start example is clear and practical.Good demonstration of the recommended pattern using
createAgentScriptPreset()withJSAstValidator.
102-114: AgentScript Preset Options table is accurate.The options table correctly documents
requireCallTool,allowedGlobals,allowArrowFunctions,allowedLoops, andadditionalDisallowedIdentifiers. This aligns with the actual API surface (unlike the README which documents the non-existentallowAsync).
116-132: Comprehensive "What AgentScript Blocks" table.The table clearly documents blocked constructs with rationale. This is excellent user-facing documentation for understanding the security model.
422-445: UnknownGlobalRule documentation is clear with appropriate caveats.Good explanation of the whitelist-based approach. The Note about flat symbol table and intentional simplification for AgentScript v1 is valuable for users who might enable user functions.
230-233: No inconsistency found. The review comment contains incorrect references.The code at lines 230-233 correctly uses
Presets.secure(). After checking the entireast-guard.mdxfile:
- All Presets method references use correct names:
secure(line 231),strict(line 225),standard(line 236)- No
Presets.secured()method exists anywhere in the code- The cited README lines 172 and 245 do not contain any Presets references
- The Presets object definition in
libs/ast-guard/src/presets/index.tsexports onlysecure,strict,standard,permissive, andcreateThe preset naming is consistent throughout the codebase and documentation.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/draft/docs/guides/ast-guard.mdx (1)
469-469: Fix typo in validation monitoring section."errorgreat" is missing a space or punctuation. Should be "error—great" or similar.
Apply this diff to fix the typo:
-- `stopOnFirstError` halts validation as soon as a rule reports an errorgreat for latency-sensitive flows. +- `stopOnFirstError` halts validation as soon as a rule reports an error—great for latency-sensitive flows.
🧹 Nitpick comments (1)
libs/ast-guard/src/__tests__/isolation-breakout.spec.ts (1)
33-497: Consider a small helper to DRY validation boilerplateMany tests repeat the same pattern of creating
code, runningvalidator.validate, and assertingresult.valid. A tiny local helper would reduce duplication and make failures easier to read, e.g.:describe('Isolation Breakout Tests', () => { let strictValidator: JSAstValidator; let agentScriptValidator: JSAstValidator; beforeEach(() => { strictValidator = new JSAstValidator(Presets.strict()); agentScriptValidator = new JSAstValidator(createAgentScriptPreset()); }); + + async function expectStrictInvalid(code: string) { + const result = await strictValidator.validate(code); + expect(result.valid).toBe(false); + return result; + } + + async function expectAgentScriptInvalid(code: string) { + const result = await agentScriptValidator.validate(code); + expect(result.valid).toBe(false); + return result; + }Then individual tests can call
await expectStrictInvalid(code)/await expectAgentScriptInvalid(code)and only assert onissueswhen needed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/draft/docs/guides/ast-guard.mdx(6 hunks)libs/ast-guard/README.md(5 hunks)libs/ast-guard/src/__tests__/isolation-breakout.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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/ast-guard/README.mdlibs/ast-guard/src/__tests__/isolation-breakout.spec.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/guides/ast-guard.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/guides/ast-guard.mdx
⏰ 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 (8)
libs/ast-guard/README.md (4)
381-442: ✅ AgentScript preset documentation is accurate.The AgentScript Preset section has been correctly updated. The non-existent
allowAsyncoption has been properly removed, and the options table now accurately reflects the actualAgentScriptOptionsinterface (requireCallTool, allowedGlobals, additionalDisallowedIdentifiers, allowArrowFunctions, allowedLoops, reservedPrefixes, staticCallTarget, callToolValidation). Examples are valid and consistent with the API.
811-837: ✅ Defense architecture documentation is clear and well-structured.The 4-layer defense model is clearly explained with concrete examples. The ASCII diagram properly uses the
textlanguage identifier (addressing the past MD040 linting issue), making the visual architecture easy to follow.
859-869: Update test count in Roadmap (line 865) to match verified total.Line 865 claims "516 tests" in the completed Roadmap. This should be updated consistently with the corrected test count from the Bank-Grade Security metrics (line 18). Both occurrences should reflect the same verified total.
1-10: ✅ Markdown formatting is now compliant.Previous linting issues (MD036 for emphasis-as-heading at line 13, MD040 for missing language identifier at line 815) have been properly resolved. Headings use correct markdown syntax, and all code fences include appropriate language identifiers.
libs/ast-guard/src/__tests__/isolation-breakout.spec.ts (1)
24-301: Comprehensive isolation surface coverage looks solidThe suite is well‑structured and clearly documents intended behavior across STRICT/STANDARD/AgentScript presets (WeakRef/FinalizationRegistry, ShadowRealm, debugger, timing, constructor‑chain escapes, reserved prefixes, etc.). Assertions are present and aligned with the comments, and the “accepted risk” cases (e.g., Date.now with STANDARD) are explicitly called out, which will help future maintainers understand security trade‑offs.
docs/draft/docs/guides/ast-guard.mdx (3)
181-187: Code Transformation example now documents all configuration options.The previous review requested including
prefixandadditionalIdentifiersin the transformation example, and this has been addressed. The example now shows all four configuration options with their defaults and descriptions, improving completeness.
199-205: Documentation of transformation benefits is clear and concise.The description of what transformations provide (main wrapper, safe callTool, safe loops, reserved prefixes) directly maps to the configuration options shown above, making it easy for users to understand the safety guarantees.
50-50: All ast-guard exports are correctly referenced—no issues found.Verification confirms that all imports mentioned in the documentation (
JSAstValidator,createAgentScriptPreset,transformAgentScript,Presets,ValidationSeverity,NoGlobalAccessRule,ReservedPrefixRule,NoCallTargetAssignmentRule,UnicodeSecurityRule,UnknownGlobalRule,DisallowedIdentifierRule,ForbiddenLoopRule,RequiredFunctionCallRule,CallArgumentValidationRule) are properly exported fromlibs/ast-guard/src/index.ts. The documentation is consistent with the public API surface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (6)
apps/demo/src/apps/crm/tools/users-create.tool.ts (1)
14-23: Consider addinglastLoginAtto the output schema for consistency.The output schema omits the
lastLoginAtfield, which is present in theUserinterface and included in similar tools (users-update,users-list). While newly created users won't have this field populated, including it as optional maintains schema consistency across the toolset.outputSchema: { user: z.object({ id: z.string(), email: z.string(), name: z.string(), role: z.enum(['admin', 'user', 'viewer']), status: z.enum(['active', 'inactive', 'pending']), createdAt: z.string(), + lastLoginAt: z.string().optional(), }), success: z.boolean(), },apps/demo/src/apps/crm/tools/activities-list.tool.ts (1)
35-38: Clarify semantics oftotalfield.The
totalfield returnsactivities.length, which is the count of returned activities (after the limit is applied). If the intent is to show the total matching records before pagination, this would require a separate count from the store. For a demo app, the current behavior is acceptable, but consider renaming tocountor documenting that it represents the returned count, not the total matching records.docs/draft/docs/guides/create-plugin.mdx (3)
128-141: Reduceanytype usage in best practices guide and consider defensive coding for dynamic access.Lines 128 and 141 cast
authInfotoanyto access user properties, and thehashObjectmethod (lines 283–288) recursively processes untrusted objects without circular-reference detection. While these examples illustrate the core concept, they could mislead readers on type safety practices.Consider showing more defensive patterns:
- Use type guards or interfaces instead of
as any- Validate object shapes before recursion
For a best practices guide, even illustrative examples should model safer patterns where feasible.
For the
getAllowedSitesmethod, consider using a type guard:- private getAllowedSites(authInfo: unknown): string[] | 'ALL' { - const user = (authInfo as any)?.user; + private getAllowedSites(authInfo: unknown): string[] | 'ALL' { + const user = this.extractUser(authInfo); + if (!user) return this.opts.demoAllowAllIfNoClaims ? 'ALL' : []; + + const sites = user.sites || user.tenants; // ... } + private extractUser(authInfo: unknown): Record<string, any> | null { + if (!authInfo || typeof authInfo !== 'object') return null; + const obj = authInfo as Record<string, any>; + return obj.user && typeof obj.user === 'object' ? obj.user : null; + }For the
hashObjectmethod, add a depth limit to prevent stack overflow on circular structures:- private hashObject(obj: Record<string, unknown>): string { + private hashObject(obj: Record<string, unknown>, depth = 0): string { + if (depth > 5) return '[circular]'; // Prevent infinite recursion const keys = Object.keys(obj).sort(); return keys.reduce((acc, key) => { const val = obj[key]; acc += key + ':'; if (typeof val === 'object' && val !== null) { - acc += this.hashObject(val as Record<string, unknown>); + acc += this.hashObject(val as Record<string, unknown>, depth + 1); } else { acc += val; } return acc + ';'; }, ''); }Also applies to: 283-288
89-125: Consider adding error handling guidance to hook examples.Both hook examples (lines 93–125 and 237–276) throw errors but don't show error handling or recovery patterns. For a best practices guide, consider adding brief guidance on:
- When to throw vs. silently skip processing
- How to handle errors from async operations (e.g., cache store failures)
- Recovery patterns for transient failures
For example, the cache read hook could show how to gracefully degrade on cache failures.
Also applies to: 237-276
369-394: Add error handling guidance for factory pattern async initialization.The factory pattern example (lines 369–394) shows
useFactorywith async initialization but doesn't demonstrate error handling. Consider adding guidance on:
- What happens if
config.getAsync()or factory function throws?- Retry/timeout patterns for async initialization failures
- Type safety for injected dependencies
Alternatively, reference a separate error handling guide if one exists.
docs/draft/docs/servers/extensibility/plugins.mdx (1)
62-127: Dynamic Plugin patterns are clear and correct. Both the simple options approach (lines 62–103) and Zod validation approach (lines 105–127) are well-structured. Consider adding a brief note about when Zod is worth the extra complexity vs. simpler validation in the constructor—this helps readers choose the right approach for their use case.Consider adding a guidance box or comment like:
When to use Zod: For plugins with many options, nested configs, or public packages where consumers set options. For simple internal plugins, constructor validation may be sufficient.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/demo/src/apps/crm/data/store.ts(1 hunks)apps/demo/src/apps/crm/index.ts(1 hunks)apps/demo/src/apps/crm/tools/activities-list.tool.ts(1 hunks)apps/demo/src/apps/crm/tools/activities-log.tool.ts(1 hunks)apps/demo/src/apps/crm/tools/activities-stats.tool.ts(1 hunks)apps/demo/src/apps/crm/tools/users-create.tool.ts(1 hunks)apps/demo/src/apps/crm/tools/users-delete.tool.ts(1 hunks)apps/demo/src/apps/crm/tools/users-get.tool.ts(1 hunks)apps/demo/src/apps/crm/tools/users-list.tool.ts(1 hunks)apps/demo/src/apps/crm/tools/users-update.tool.ts(1 hunks)apps/demo/src/main.ts(1 hunks)docs/draft/docs.json(2 hunks)docs/draft/docs/guides/create-plugin.mdx(1 hunks)docs/draft/docs/servers/extensibility/plugins.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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/crm/tools/users-create.tool.tsapps/demo/src/apps/crm/index.tsapps/demo/src/apps/crm/tools/users-delete.tool.tsapps/demo/src/apps/crm/tools/users-get.tool.tsapps/demo/src/apps/crm/tools/users-list.tool.tsapps/demo/src/apps/crm/tools/users-update.tool.tsapps/demo/src/apps/crm/tools/activities-list.tool.tsapps/demo/src/apps/crm/tools/activities-stats.tool.tsapps/demo/src/apps/crm/tools/activities-log.tool.tsapps/demo/src/apps/crm/data/store.tsapps/demo/src/main.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/guides/create-plugin.mdxdocs/draft/docs/servers/extensibility/plugins.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/guides/create-plugin.mdxdocs/draft/docs.jsondocs/draft/docs/servers/extensibility/plugins.mdx
🧬 Code graph analysis (6)
apps/demo/src/apps/crm/tools/users-create.tool.ts (1)
apps/demo/src/apps/crm/data/store.ts (1)
CrmStore(137-226)
apps/demo/src/apps/crm/tools/users-list.tool.ts (1)
apps/demo/src/apps/crm/data/store.ts (2)
User(5-13)CrmStore(137-226)
apps/demo/src/apps/crm/tools/users-update.tool.ts (1)
apps/demo/src/apps/crm/data/store.ts (1)
CrmStore(137-226)
apps/demo/src/apps/crm/tools/activities-list.tool.ts (1)
apps/demo/src/apps/crm/data/store.ts (1)
Activity(15-22)
apps/demo/src/apps/crm/tools/activities-stats.tool.ts (1)
apps/demo/src/apps/crm/data/store.ts (1)
CrmStore(137-226)
apps/demo/src/apps/crm/tools/activities-log.tool.ts (1)
apps/demo/src/apps/crm/data/store.ts (1)
CrmStore(137-226)
⏰ 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 (25)
apps/demo/src/main.ts (1)
5-9: LGTM!The CRM app import and integration follow the established pattern used by other demo apps.
apps/demo/src/apps/crm/index.ts (1)
1-43: Well-structured CRM app configuration.The app is cleanly organized with clear separation of user and activity tools. The CodeCallPlugin configuration with
topK: 10is appropriate for the 8 registered tools, and the inline comments helpfully document the plugin behavior.apps/demo/src/apps/crm/tools/activities-stats.tool.ts (1)
1-26: LGTM!Clean implementation that correctly delegates to
CrmStore.activities.getStats. The output schema accurately reflects the store's return type.apps/demo/src/apps/crm/tools/users-get.tool.ts (1)
8-11: Validation approach is acceptable for demo purposes.The schema declares both
idand.refine()for better client-side validation feedback.Also applies to: 28-31
apps/demo/src/apps/crm/tools/activities-log.tool.ts (2)
46-49: Minor: Update result not checked (acceptable for demo).The
CrmStore.users.update()call doesn't check its return value. In a production scenario with concurrent access, the user could be deleted between the existence check (line 34) and the update (line 48). For a demo app with in-memory storage, this TOCTOU window is negligible.
1-55: LGTM overall!The tool implementation is well-structured with proper user validation, activity creation, and the nice touch of updating
lastLoginAton login activities. Schema definitions are thorough and match the CrmStore's Activity type.apps/demo/src/apps/crm/tools/users-list.tool.ts (1)
1-35: LGTM!The implementation correctly delegates to
CrmStore.users.list()with the appropriate filter parameters. The input and output schemas align well with theUserinterface and the store's API signature. The optionallastLoginAtfield in the output schema properly matches the interface definition.apps/demo/src/apps/crm/tools/users-update.tool.ts (1)
1-61: LGTM!The implementation properly validates user existence and email uniqueness before performing the update. The defensive checks align well with the expected behavior of a CRM update operation. The nullable user in the output schema correctly handles edge cases.
apps/demo/src/apps/crm/tools/users-create.tool.ts (1)
33-49: LGTM!The email uniqueness check and user creation logic are correctly implemented. The defaults for
roleandstatusare sensible, and the return structure properly reflects the successful creation.apps/demo/src/apps/crm/tools/users-delete.tool.ts (1)
1-30: LGTM!The implementation correctly validates user existence before deletion. The defensive handling of the delete result is appropriate, even though the failure path is unlikely to be reached after the existence check passes.
apps/demo/src/apps/crm/tools/activities-list.tool.ts (1)
1-26: LGTM!The tool decorator, input schema, and output schema are well-defined and align correctly with the
Activityinterface andCrmStore.activities.listAPI.apps/demo/src/apps/crm/data/store.ts (4)
5-22: Well-designed interfaces for the demo.The User and Activity interfaces are clean, type-safe, and appropriate for a CRM demo. The use of string literal unions for
role,status, andtypeprovides good type safety.
25-129: Appropriate sample data for demo purposes.The sample users and activities provide realistic test data that effectively demonstrates the CRM functionality.
139-181: User operations implemented correctly.The CRUD operations for users are well-implemented:
listprovides flexible filteringgetuses efficient Map lookupcreategenerates sequential IDs properlyupdatesupports partial updatesdeleteworks as expected
185-225: Activity operations work correctly for demo purposes.The activity operations provide appropriate functionality:
listwith flexible filtering, sorting, and paginationcreategenerates sequential IDsgetStatsprovides useful aggregationThe implementation is suitable for a demo application.
docs/draft/docs/guides/create-plugin.mdx (4)
245-261: Verifythis.get()andtool.safeParseOutput()API methods exist in the SDK.The cache plugin example uses
this.get(CacheStoreToken)(line 246) andtool.safeParseOutput()(line 252) without verifying these methods are part of the public DynamicPlugin and ToolEntry APIs. Please confirm these methods exist and are intended for public use.If
this.get()is indeed the API for retrieving providers, consider adding a note about provider resolution and fallback patterns (e.g., what happens if the provider is not found?). Iftool.safeParseOutput()is a real method, briefly document its return type and error handling.Also applies to: 271-276
350-361: VerifydynamicProvidersstatic method signature and documentation.Line 350 shows
static override dynamicProviders(options: MyPluginOptionsInput): ProviderType[], which assumes:
- This is a valid override of a base
DynamicPluginmethodProviderTypeis the correct return type- The method is called during plugin initialization
Please verify this signature matches the actual SDK and add a note explaining when
dynamicProvidersis invoked (e.g., duringMyPlugin.init()execution).
403-431: Verify hook priorities and stage names match actual SDK conventions.The Hook Reference table (lines 403–431) documents:
- Supported hooks:
ToolHookandListToolsHook- Flow names and stages (e.g.,
tools:call-tool,tools:list-tools)- Priority conventions (100-500 for security, etc.)
Please confirm these stage names and priority conventions match the actual SDK implementation. If they differ, update the guide accordingly.
455-470: Add reference to actual CodeCall plugin implementation and link to advanced patterns.The "Best Practices" and "Complete Example" sections (lines 455–470) suggest readers see the CodeCall and CachePlugin for complete examples. This is good, but ensure:
- Links or file paths to these plugins are provided (or will be added during generation)
- A corresponding CodeCall plugin documentation page exists (per docs.json entry at line 125)
- Any patterns used in CodeCall that differ from this guide are clearly explained in the CodeCall docs
docs/draft/docs.json (2)
124-126: Confirm plugin pages list is in the intended order.The plugins pages list now includes three entries:
overview,cache-plugin,codecall-plugin(line 125). Verify this order matches the documentation structure and is the intended user journey (e.g., overview first, then specific plugins in priority order).
125-125: All referenced documentation files exist with correct paths in docs.json.Verification confirms all three documentation entries referenced in the navigation configuration exist and are correctly mapped:
docs/plugins/codecall-plugin→docs/draft/docs/plugins/codecall-plugin.mdx✓docs/guides/create-plugin→docs/draft/docs/guides/create-plugin.mdx✓blog/11-2025/codecall-plugin→docs/draft/blog/11-2025/codecall-plugin.mdx✓Note: The original review comment expected the blog file at
docs/draft/blogs/11-2025/codecall-plugin.mdx(plural), but the correct directory isdocs/draft/blog/(singular). The docs.json configuration correctly referencesblog/11-2025/codecall-plugin.docs/draft/docs/servers/extensibility/plugins.mdx (4)
1-25: Clear introduction and solid Basic Plugin template. The intro is concise, and the@Plugindecorator example shows the essential fields well. Good starting point for progressive complexity.
27-60: Three attachment patterns are well-explained and practical. The progression from static to factory-based init is clear, and Option 3 nicely demonstrates the injected config pattern used by CodeCallPlugin. This section sets up well for the DynamicPlugin details below.
232-274: Hook reference is comprehensive and well-structured. The tables clearly map stages to phases, and the Will/Did timing explanation with code examples is excellent.
296-325: Metadata extension and composition sections are well-written. The global interface augmentation pattern (lines 301–305) is a clever way to extend tool metadata, and the practical tip about using plugins for org-wide concerns is valuable guidance. The depth-first composition model is clearly explained.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/demo/src/apps/crm/tools/users-create.tool.ts(1 hunks)docs/draft/docs/guides/create-plugin.mdx(1 hunks)docs/draft/docs/servers/extensibility/plugins.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/demo/src/apps/crm/tools/users-create.tool.ts
- docs/draft/docs/guides/create-plugin.mdx
🧰 Additional context used
📓 Path-based instructions (2)
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/extensibility/plugins.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/extensibility/plugins.mdx
⏰ 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 (2)
docs/draft/docs/servers/extensibility/plugins.mdx (2)
163-239: Excellent: CachePluginOptions is now fully defined.The previous review flagged that
CachePluginOptionswas incomplete. This issue is now resolved—the interface is properly defined at lines 171–174 and used consistently throughout the example. All references (line 193, 208, 230) align correctly.
306-310: Good: Info box addresses the previous type reference concern.This clarification appropriately resolves the previous review issue about non-exported internal types (
InitOptions,PluginReturn). The note tells developers they needn't import or reference these types, which is practical for the common use case shown in lines 39–57 (passing the result directly to the plugins array).
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.