Tier 4: typed signatures, dotted naming, modern MCP fields, and Arazzo workflows - #28
Merged
Conversation
… reserved names, and depth wiring in type signatures
…ed list, and resolver this-binding
…itation descriptors
…llowlist, and reserved namespace protection
…, dotted names, cross-document dependsOn, and reusable re-validation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR adds Arazzo workflow conversion, runtime-expression parsing, TypeScript tool contracts, dotted naming, security elicitations, MCP metadata, icons, wire-header preservation, public exports, documentation, and tests. ChangesMCP generation extensions
Estimated code review effort: 5 (Critical) | ~120 minutes Mergeability Score: 🔵 Low · up to The PR adds substantial new API and workflow capabilities, but the documentation example for accessing tool metadata and icons remains incomplete and could mislead users. Merge is reasonable with explicit owner follow-up to correct that example. Sequence Diagram(s)sequenceDiagram
participant Caller
participant fromArazzo
participant generateTools
participant parseRuntimeExpression
Caller->>fromArazzo: Submit Arazzo document and OpenAPI sources
fromArazzo->>generateTools: Resolve source operations and schemas
fromArazzo->>parseRuntimeExpression: Parse workflow expressions
parseRuntimeExpression-->>fromArazzo: Return expression ASTs
fromArazzo-->>Caller: Return consolidated MCP workflow tools
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/generator.ts (1)
236-260: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider sharing one icon-source validator.
iconsFromInfoLogorepeats thehttps:/data:scheme rule thatisAllowedIconSrcimplements insrc/annotations.ts. Export one helper and call it from both places, so the two paths cannot drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/generator.ts` around lines 236 - 260, The icon scheme validation is duplicated between iconsFromInfoLogo and isAllowedIconSrc. Export the existing validator from annotations.ts, then reuse it in iconsFromInfoLogo instead of maintaining a separate https:/data: check, preserving the current accepted-source behavior in both paths.src/__tests__/integration.spec.ts (1)
743-744: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer a static import of the barrel over
require.
require('../index')returns an untyped module and can trip@typescript-eslint/no-require-importsin a TypeScript spec. A namespace import keeps the public-entrypoint-only rule and restores type checking on the asserted fields.♻️ Proposed refactor
-describe('Tier 4 surface through the entrypoint', () => { - /* eslint-disable `@typescript-eslint/no-explicit-any` */ - const lib = require('../index'); +import * as lib from '../index'; + +describe('Tier 4 surface through the entrypoint', () => { + /* eslint-disable `@typescript-eslint/no-explicit-any` */Move the import to the top of the file with the other imports.
As per coding guidelines: "Integration tests:
src/__tests__/integration.spec.ts(full pipeline, imports from entrypoint only)". A static import of../indexstill satisfies that rule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/integration.spec.ts` around lines 743 - 744, Replace the dynamic require in integration.spec.ts with a top-level namespace import from ../index, remove the explicit-any ESLint suppression, and update usages as needed so the asserted public-entrypoint fields retain static type checking.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/annotations.md`:
- Line 45: Update the documentation references for extension overrides: in
docs/annotations.md, revise the x-frontmcp section statement and the
extractExtensionOverrides result to include meta and icons; in
docs/x-frontmcp.md, extend the FrontMcpExtensionData type definition with meta
and icons.
In `@docs/naming-strategies.md`:
- Around line 113-118: Update the documented NamingStrategy interface block to
match the exported type: make conflictResolver optional with the default
location-prefix behavior, and change toolNameGenerator to accept the fourth
operation argument. Keep the surrounding naming strategy documentation
unchanged.
In `@src/__tests__/annotations.spec.ts`:
- Around line 277-282: Update the test “cleanses meta arrays and scalars in
place” to construct the array item via JSON.parse, matching the setup used by
the nearby test around line 267, so __proto__ is an own key and the assertion
verifies cleanseMeta removes it from nested array objects.
In `@src/arazzo.ts`:
- Around line 396-401: Guard both document-supplied component lookups in
src/arazzo.ts:396-401 and src/arazzo.ts:424-428 with
Object.prototype.hasOwnProperty.call and require the resolved target to be an
object before accepting it. Update the $components parameters lookup around the
existing expectedGroup/name logic and the components.inputs lookup before
toJsonSchema, preserving the existing unknown-reference ArazzoError behavior for
inherited, missing, or non-object values.
In `@src/type-signature.ts`:
- Around line 365-374: Update the empty-properties branch in paramList to
recognize root oneOf, anyOf, allOf, enum, or const schemas as data-carrying and
return an input parameter instead of '()'. Preserve the existing no-argument
behavior only for genuinely closed, empty object roots, and add a test covering
a root oneOf input schema through the standalone entrypoint.
---
Nitpick comments:
In `@src/__tests__/integration.spec.ts`:
- Around line 743-744: Replace the dynamic require in integration.spec.ts with a
top-level namespace import from ../index, remove the explicit-any ESLint
suppression, and update usages as needed so the asserted public-entrypoint
fields retain static type checking.
In `@src/generator.ts`:
- Around line 236-260: The icon scheme validation is duplicated between
iconsFromInfoLogo and isAllowedIconSrc. Export the existing validator from
annotations.ts, then reuse it in iconsFromInfoLogo instead of maintaining a
separate https:/data: check, preserving the current accepted-source behavior in
both paths.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15db536f-1e29-4eab-9858-3249ce09a51d
📒 Files selected for processing (31)
CLAUDE.mdREADME.mddocs/annotations.mddocs/api-reference.mddocs/arazzo.mddocs/configuration.mddocs/modern-mcp-fields.mddocs/naming-strategies.mddocs/type-signatures.mddocs/x-frontmcp.mdjest.config.jssrc/__tests__/annotations.spec.tssrc/__tests__/arazzo.spec.tssrc/__tests__/elicitation.spec.tssrc/__tests__/errors.spec.tssrc/__tests__/generator.spec.tssrc/__tests__/integration.spec.tssrc/__tests__/naming-presets.spec.tssrc/__tests__/type-signature.spec.tssrc/annotations.tssrc/arazzo-expressions.tssrc/arazzo-types.tssrc/arazzo.tssrc/elicitation.tssrc/errors.tssrc/generator.tssrc/index.tssrc/naming-presets.tssrc/parameter-resolver.tssrc/type-signature.tssrc/types.ts
…okup guards, composed input roots, and doc sync
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/x-frontmcp.md`:
- Around line 188-189: Update the FrontMcpExtensionData access example to
destructure both meta and icons alongside the existing fields, keeping the
example synchronized with the documented type definition.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dfb6844d-ccbd-47b6-bf3b-b8166d61144a
📒 Files selected for processing (10)
docs/annotations.mddocs/naming-strategies.mddocs/x-frontmcp.mdsrc/__tests__/annotations.spec.tssrc/__tests__/arazzo.spec.tssrc/__tests__/type-signature.spec.tssrc/annotations.tssrc/arazzo.tssrc/generator.tssrc/type-signature.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/tests/annotations.spec.ts
- src/annotations.ts
- docs/annotations.md
- src/type-signature.ts
- src/generator.ts
- src/arazzo.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tier 4 of the roadmap — the strategic bets. Four features, each landed as a single commit with its tests and docs, each followed by an adversarial review pass whose confirmed findings landed as a
fix:commit. 1,029 tests, 100% statements/branches/functions/lines enforced throughout.T4-1 — TypeScript call signatures (
emitTypeSignatures)generateTools({ emitTypeSignatures: true })renders each tool's call contract asmetadata.typescript = { signature, declaration }— a one-line arrow type plus a self-contained declaration block (JSDoc from schema descriptions, named<ToolName>Input/<ToolName>Outputtypes,declare function). Computed on the FINAL schemas (post formats/depth/trims/client-target); recomputed on collision-dedup renames; return types are the unwrapped response (FrontMCP's{status, ok, data, error}wrapping happens downstream). Also exported standalone (emitToolTypeScript,toPascalIdentifier). Review hardening: union/intersection roots emittypealiases (never invalidinterface X {...} | {...}),*/*content-types and statuses escaped in variant comments, reserved-word function names suffixed (delete→delete_), printer depth followsmaxSchemaDepth, non-finite literals degrade tonumber, crafted cyclic type arrays can't overflow the stack.T4-1b —
dottedNamingpresetOpt-in naming preset producing two-segment
ns.methodnames bindable as CodeCall sandbox namespaces (await billing.listInvoices({...})): first tag → first path segment →api, exactly one dot, both halves identifier-safe.NamingStrategy.conflictResolveris now optional (defaults to the location-prefix resolver) andtoolNameGeneratorreceives the operation as a 4th argument. Review hardening: digit-leading namespaces get a letter guard (3rd-party→n3rd_party— a_guard would be trimmed by name normalization), the reserved list mirrors CodeCall's actual sandbox globals, and class-based strategies keep theirthisbinding.T4-3 — Modern-spec surface
_metaon tools: opt-inemitMetaemitsdev.agentfront.openapi/operation(path/method/operationId/tags/deprecated/specTitle/specVersion);x-mcp.meta/x-frontmcp.metapass through even without the flag. The reserveddev.agentfront.openapi/namespace cannot be spoofed by extensions, and pollution-gadget keys (__proto__/constructor/prototype) are stripped recursively (confirmed exploitable via both JSON and YAML parse paths before the fix).iconson tools fromx-frontmcp.icons/x-mcp.icons, plus opt-ininfo['x-logo']fallback (inheritDocumentIcons);srcrestricted tohttps:/data:per the documented contract.x-mcp-headermarkers on every header-located input property (always on; survives conflict renames and all client targets) so generic bridges can route headers without the mapper.deriveSecurityElicitations(tool)— MCP-elicitation-shaped{ message, requestedSchema }credential descriptors per security scheme (bearer/basic/digest/apiKey/oauth2/oidc), pure data.T4-2 —
fromArazzo()(full Arazzo 1.0)One consolidated
McpOpenAPIToolper workflow: inputs → input schema, outputs → best-effort derived output schema, and a pure serializable IR onmetadata.workflow— each operation step embeds its resolvedinputSchema/outputSchema/mapper/security/servers so executors need no second spec pass (operation.mapperfeedsbuildHttpRequestdirectly). Multi-source (sourcesmap; URLs never fetched),operationId/operationPath/nested-workflowIdsteps, full runtime-expression AST (incl.$message.),$componentsinlining (dotted names supported), reusable objects fully re-validated after resolution,dependsOn/invocation cycle detection with cross-documentdependsOnexpressions carried verbatim, request bodies with pointer-keyed expression substitution lists, and the same schema pipeline asgenerateTool(ArazzoGenerateOptions, includingemitTypeSignatures). Documents are normalized via a JSON round-trip: YAML anchors expand (every aliased expression occurrence is recorded), YAML-only scalars become JSON forms, cyclic/absurdly-deep inputs fail asArazzoErrorinstead of crashing. NewArazzoErrorcarries a JSON-Pointerpathfor every failure.New docs
docs/type-signatures.md,docs/modern-mcp-fields.md,docs/arazzo.md, plus updates to naming-strategies, configuration, api-reference, x-frontmcp, annotations, README, CLAUDE.md.Test plan
yarn test:coverage— 1,029 tests, 100% statements/branches/functions/lines (enforced), exit 0npx tsc -p tsconfig.lib.json --noEmit— cleanyarn build— CJS + ESM + declarations cleanSummary by CodeRabbit
namespace.methodnaming presets with collision handling.