Skip to content

Conversation

@dimaMachina
Copy link
Collaborator

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Dec 22, 2025

🦋 Changeset detected

Latest commit: 5708715

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@inkeep/agents-manage-mcp Patch
@inkeep/ai-sdk-provider Patch
@inkeep/create-agents Patch
@inkeep/agents-core Patch
@inkeep/agents-sdk Patch
@inkeep/agents-manage-api Patch
@inkeep/agents-manage-ui Patch
@inkeep/agents-run-api Patch
@inkeep/agents-cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Dec 22, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
agents-manage-api Error Error Dec 22, 2025 9:49pm
agents-manage-ui Ready Ready Preview, Comment Dec 22, 2025 9:49pm
agents-run-api Ready Ready Preview, Comment Dec 22, 2025 9:49pm
1 Skipped Deployment
Project Deployment Review Updated (UTC)
agents-docs Skipped Skipped Dec 22, 2025 9:49pm

@claude
Copy link

claude bot commented Dec 22, 2025

Claude finished @dimaMachina's task —— View job


TypeScript Architecture Review

Todo List:

  • Read repository setup and guidelines
  • Analyze the verbatimModuleSyntax compiler option change
  • Review the code changes in types.ts
  • Assess system-wide impact and potential issues
  • Evaluate build/deployment implications
  • Provide comprehensive architectural feedback

Summary

This PR introduces verbatimModuleSyntax: true across all TypeScript configurations in the monorepo. This is a high-impact architectural change that enforces strict ES module syntax and has both benefits and potential risks.

Key Changes Analysis

1. Compiler Option Addition

The PR adds "verbatimModuleSyntax": true to 22 tsconfig.json files across the entire monorepo, including:

  • Core packages (@inkeep/agents-core, @inkeep/agents-sdk, etc.)
  • Applications (agents-run-api, agents-manage-api, etc.)
  • Templates and examples

2. Import/Export Syntax Changes

File: agents-run-api/src/a2a/types.ts:1-5

// Before:
import {
  AgentCard,
  type Artifact,
  type Message,
  type Task,
  type TaskState,
} from '@inkeep/agents-core';
export { AgentCard };

// After:
import type { AgentCard, Artifact, Message, Task, TaskState } from '@inkeep/agents-core';
export type { AgentCard };

Architecture Review

Positive Aspects

  1. Type Safety Enhancement: verbatimModuleSyntax prevents accidental runtime imports of type-only constructs, improving bundle safety
  2. Consistent Module Semantics: Enforces explicit distinction between type imports and value imports across the entire codebase
  3. Future-Proofing: Aligns with TypeScript's direction toward stricter module handling
  4. Monorepo-Wide Consistency: Applied systematically across all packages

⚠️ Critical Concerns & Potential Issues

1. Breaking Change Risk - HIGH SEVERITY

  • This compiler option can introduce breaking changes in existing code that mixed type/value imports
  • Impact: The failing Vercel deployment for agents-manage-api suggests there may be additional import/export issues not addressed in this PR
  • Recommendation: Need comprehensive testing across all packages to identify other affected files

2. Incomplete Migration - MEDIUM SEVERITY

  • Only one file (types.ts) was modified for import syntax, but verbatimModuleSyntax affects the entire codebase
  • Missing Analysis: The PR should identify ALL files with mixed import statements that need updating
  • Risk: Runtime errors in production if type imports are used where values are expected

3. Build System Impact - MEDIUM SEVERITY

  • The Vercel deployment failure for agents-manage-api indicates potential build issues
  • Module Resolution: With moduleResolution: "bundler" + verbatimModuleSyntax: true, bundlers must handle the strict separation
  • Bundler Compatibility: Need to verify all bundlers (Vite, Next.js, etc.) properly handle this configuration

4. Cross-Package Dependencies - MEDIUM SEVERITY

  • The monorepo has complex internal dependencies (@inkeep/agents-core exports used by multiple packages)
  • Re-export Patterns: The change from export { AgentCard } to export type { AgentCard } could break downstream consumers expecting a runtime value
  • Type vs Value Confusion: Need to audit all re-exports to ensure correct type/value classification

5. Developer Experience Concerns - LOW TO MEDIUM SEVERITY

  • Learning Curve: Developers must now distinguish between import type and import more carefully
  • IDE Support: Ensure all developers have TypeScript language servers that properly support this feature
  • Error Messages: May produce confusing errors for developers unfamiliar with this strict mode

Specific Code Review - agents-run-api/src/a2a/types.ts

Potential Breaking Change

The change from export { AgentCard } to export type { AgentCard } is potentially breaking:

// Before - AgentCard available as runtime value
export { AgentCard };

// After - AgentCard only available as type
export type { AgentCard };

Impact Assessment Required:

  • Search for usages: import { AgentCard } in consuming code
  • If AgentCard is used at runtime (not just for typing), this breaks those usages
  • Need to verify AgentCard is only used for type annotations

System-Wide Recommendations

1. Immediate Actions - REQUIRED

# Find all mixed import statements that need updating
grep -r "import.*{.*type\|import.*{[^}]*," --include="*.ts" --include="*.tsx" .

2. Migration Strategy

  1. Audit Phase: Identify all files needing import syntax updates
  2. Staged Rollout: Consider enabling per-package rather than monorepo-wide initially
  3. Testing: Comprehensive integration testing across all packages
  4. Documentation: Update contribution guidelines for new import patterns

3. Build Verification

  • Fix the failing agents-manage-api deployment before merge
  • Verify all Vercel deployments succeed
  • Test local builds for all packages: npm run build in each package

4. Long-term Considerations

  • ESLint Rules: Add rules to enforce proper import type usage
  • Pre-commit Hooks: Validate import syntax compliance
  • Template Updates: Ensure create-agents-template reflects new patterns

Customer Impact Assessment

Runtime Compatibility

  • Risk: If AgentCard is used at runtime by customers, the export change breaks their integration
  • API Stability: This could constitute a breaking change requiring major version bump
  • Migration Guide: Need clear guidance for customers on import pattern changes

Bundle Size Impact

  • Positive: Better tree-shaking potential with explicit type-only imports
  • Build Performance: May improve build times with clearer module boundaries

Verdict

This is a well-intentioned architectural improvement but appears to be an incomplete migration with significant risk of breaking changes.

Recommendation: Request Changes

  1. Complete the migration by identifying and updating ALL files with mixed imports
  2. Fix the deployment failure in agents-manage-api
  3. Verify AgentCard usage isn't broken by the export type change
  4. Consider phased rollout rather than monorepo-wide change
  5. Add comprehensive tests covering import/export scenarios

The TypeScript 5.x versions across the monorepo support this feature well, but the implementation needs completion before it's safe to merge.


@vercel vercel bot temporarily deployed to Preview – agents-docs December 22, 2025 21:46 Inactive
@dimaMachina dimaMachina merged commit b231869 into main Dec 22, 2025
4 of 8 checks passed
@dimaMachina dimaMachina deleted the verbatimModuleSyntax branch December 22, 2025 21:47
@github-actions
Copy link
Contributor

🔎💬 Inkeep AI search and chat service is syncing content for source 'Inkeep Agent Framework Docs'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants