feat(create-app): Add protocol option and improve CLI scaffolding - #78
Conversation
- Replaced `prompts` with `@inquirer/prompts` and updated to version 7.10.1. - Added `figlet` as a dependency and updated to version 1.9.4. - Introduced `@inquirer/testing` and `@types/figlet` as devDependencies. - Refactored CLI to utilize `@inquirer/prompts` for user interactions. - Enhanced project scaffolding to include testing setup files and dependencies. - Updated package.json scripts to use npm consistently for installations. - Added Vitest configuration and initial test setup files for improved testing capabilities. All changes maintain existing functionality while improving code clarity and structure.
- Introduced a new `protocol` option in the CLI, allowing users to specify either "mcp" or "openai" when creating applications. - Updated argument parsing to default the protocol to "mcp" if not specified. - Enhanced interactive prompts to include protocol selection. - Updated relevant scaffolding functions to handle the new protocol parameter. - Added tests to ensure proper parsing and validation of the protocol option. These changes improve flexibility in application setup and enhance user experience during project initialization.
- Replaced `sendMessage` with `sendFollowUpMessage` to align with updated OpenAI API. - Changed `openLink` to use `openExternal` for better external link handling. - Updated `setState` to utilize `setWidgetState`, enhancing state management consistency. These changes ensure compatibility with the latest OpenAI features and improve the overall functionality of the adapter.
- Updated the server start logic to only execute when the NODE_ENV is not set to "test", preventing unnecessary server initialization during testing. - Enhanced the integration test to check for the presence of the "test:watch" script in package.json, ensuring proper test setup. These changes improve the testing experience by avoiding server startup in test scenarios.
- Streamlined the `validateProjectName` function by removing unnecessary checks and simplifying the scoped package validation logic. - Refactored the `parseArgs` function to eliminate the initial result object, directly returning the constructed options, enhancing clarity and reducing code complexity. These changes improve code readability and maintainability in the CLI package.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Scaffolder
participant TemplateGen
participant FS
User->>CLI: run create-app (args or interactive)
CLI->>User: (if interactive) render figlet header, ask prompts (name, template, protocol, vercel, etc.)
CLI->>Scaffolder: call scaffoldProject(options with protocol)
Scaffolder->>TemplateGen: generate template files (pass protocol)
TemplateGen->>FS: write project files (server, ui, tests, package.json)
Scaffolder->>FS: run installer messaging (npm) / skip install per options
Scaffolder->>CLI: return success and final instructions
CLI->>User: print completion message (includes protocol)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code ReviewI've reviewed PR #78 and have the following feedback: ✅ Strengths
🔍 Code Quality Issues1. Potential issue with protocol configThe generated server code includes 2. Package manager commentThe comment "Always use npm for standalone projects" could be more detailed to explain why (avoiding workspace conflicts in pnpm monorepo). 3. Minor: Code simplificationThe 🐛 Potential Bugs1. Test flakiness fixThe timestamp comparison fix is good, but assumes timestamps are the ONLY potentially different values. Consider using a more explicit approach or mocking Date.now() if other time-sensitive data is added. ⚡ Performance ConsiderationsNetwork calls during scaffolding could be slow. Consider adding a timeout and progress indicator when fetching package versions. 📝 Additional ObservationsOpenAI adapter refactoring: Method signature updates look correct and align with latest OpenAI Apps SDK ✅ Template improvements: Better TypeScript typing, integration tests, improved UI styling, and dark mode support are excellent additions ✅ 📊 Test Coverage
🎯 Recommendations
✨ Overall AssessmentSolid PR with valuable functionality. Well-structured, tested, and follows conventions. Main concern is verifying the config.protocol support. Recommendation: ✅ Approve with minor revisions Review generated by Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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)
packages/ui/src/adapters/openai.ts (1)
525-533: Fix incorrect type assertion and error handling inopenLinkmethod.The type assertion is incorrect. The OpenAI API
openExternalmethod returnsvoid, notPromise<void>. Remove theawaitoperator and the Promise-based type cast. Additionally, this method must useAppErrorandErrorCodefrom@mcp-apps-kit/corefor error handling per the codebase guidelines, rather than throwing generic errors.Update the method to:
- Correct the type signature:
openai.openExternal({ href: url })- Remove the
await(the method is synchronous)- Add proper error handling using
AppErrorandErrorCodeif needed
🤖 Fix all issues with AI agents
In @packages/create-app/package.json:
- Around line 51-57: Update the dependency version strings in package.json for
the listed packages to the latest stable releases: change "@inquirer/prompts"
from "^7.5.0" to "^7.8.6", change "figlet" from "^1.8.0" to "^1.9.4", and change
"@inquirer/testing" from "^2.1.0" to "^2.1.47" (leave "@types/figlet" as-is
since it's already current); after updating these entries, run your package
manager to refresh the lockfile (e.g., npm install or pnpm install) and run the
test/build scripts to verify nothing breaks.
In @packages/ui/src/adapters/openai.ts:
- Around line 514-521: The sendMessage method's parameter currently declares an
unused property `type`; rename it to `_type` in the signature (change content: {
type: string; text: string } to content: { _type: string; text: string }) so the
unused variable is prefixed with an underscore, and keep the rest of sendMessage
(including the call to openai.sendFollowUpMessage with content.text) unchanged;
if message-type validation is needed instead of ignoring it, mirror the
validation logic from the mcp.ts adapter for `_type` and implement appropriate
handling rather than simply renaming.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
packages/create-app/package.jsonpackages/create-app/src/cli.tspackages/create-app/src/index.tspackages/create-app/tests/integration/scaffold.test.tspackages/create-app/tests/unit/cli.test.tspackages/ui/src/adapters/openai.tspackages/ui/tests/contract/client.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use Strict TypeScript mode: noanytypes, useunknownand narrow types instead
Remove all unused variables or prefix them with underscore (_)
Useexport typefor type-only exports
Files:
packages/create-app/src/index.tspackages/ui/src/adapters/openai.tspackages/create-app/src/cli.tspackages/create-app/tests/unit/cli.test.tspackages/ui/tests/contract/client.test.tspackages/create-app/tests/integration/scaffold.test.ts
**/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Export public API only through
index.tsfiles
Files:
packages/create-app/src/index.ts
packages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
AppErrorandErrorCodefrom@mcp-apps-kit/corefor error handling
Files:
packages/create-app/src/index.tspackages/ui/src/adapters/openai.tspackages/create-app/src/cli.tspackages/create-app/tests/unit/cli.test.tspackages/ui/tests/contract/client.test.tspackages/create-app/tests/integration/scaffold.test.ts
**/tests/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files in
tests/directory mirroring source structure with subdirectories for unit/, integration/, and contract/ tests
Files:
packages/create-app/tests/unit/cli.test.tspackages/ui/tests/contract/client.test.tspackages/create-app/tests/integration/scaffold.test.ts
🧠 Learnings (3)
📚 Learning: 2026-01-09T14:18:43.501Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T14:18:43.501Z
Learning: Applies to packages/**/*.ts : Use `AppError` and `ErrorCode` from `mcp-apps-kit/core` for error handling
Applied to files:
packages/create-app/src/index.tspackages/create-app/tests/integration/scaffold.test.ts
📚 Learning: 2026-01-09T14:18:43.501Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T14:18:43.501Z
Learning: Applies to **/tests/**/*.test.ts : Place test files in `tests/` directory mirroring source structure with subdirectories for unit/, integration/, and contract/ tests
Applied to files:
packages/create-app/src/index.tspackages/create-app/tests/integration/scaffold.test.ts
📚 Learning: 2026-01-09T14:18:43.501Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T14:18:43.501Z
Learning: Run full checks before PR: `pnpm build && pnpm test && pnpm lint && pnpm typecheck`
Applied to files:
packages/create-app/src/index.ts
🧬 Code graph analysis (2)
packages/create-app/tests/unit/cli.test.ts (1)
packages/create-app/src/cli.ts (1)
parseArgs(74-133)
packages/create-app/tests/integration/scaffold.test.ts (1)
packages/create-app/src/index.ts (1)
scaffoldProject(1195-1258)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: claude-review
- GitHub Check: test-and-lint
🔇 Additional comments (30)
packages/ui/tests/contract/client.test.ts (1)
139-142: LGTM! Effective fix for flaky timestamp comparisons.The destructuring approach correctly isolates and ignores the non-deterministic
timestampfield while comparing the rest of the result structure. The unused timestamp variables are properly prefixed with underscores per coding guidelines.Also applies to: 156-159
packages/ui/src/adapters/openai.ts (2)
663-667: LGTM! Cleaner implementation.The simplified return statement is more concise while maintaining the same functionality of wrapping the output with the tool name when available.
566-572: OpenAI API method implementation is correct.The code properly calls
openai.setWidgetState, which aligns with the official OpenAI ChatGPT Apps SDK. The adapter'ssetStatemethod correctly wraps the SDK'ssetWidgetStateAPI and includes appropriate type checking before invocation.packages/create-app/src/index.ts (13)
18-18: LGTM: Protocol option properly typed.The optional
protocolfield with union type"mcp" | "openai"aligns with the PR objectives and provides type safety.
33-33: LGTM: Testing version tracking added.The
testingfield is properly added to support the new test scaffolding feature.
77-81: LGTM: Protocol parameter with sensible default.The protocol parameter with a default value of
"mcp"ensures backward compatibility while enabling the new feature.
252-256: LGTM: Server start properly gated for test environment.Wrapping the server start in
if (process.env.NODE_ENV !== "test")prevents port conflicts during tests, aligning with the PR objectives.
258-259: LGTM: App export enables testing.Exporting the app instance allows the generated test files to import and test the application, which is essential for the new test scaffolding feature.
273-294: LGTM: Tool result handling supports multiple formats.The code properly handles both wrapped (
{ hello: {...} }) and unwrapped result formats with clear type definitions and helpful comments.
522-534: LGTM: Vitest configuration is well-structured.The test configuration with 30-second timeouts and node environment is appropriate for integration tests.
535-542: LGTM: Test setup file is clean and simple.The setup file properly initializes Vitest matchers from the testing package.
543-608: LGTM: Integration test template provides comprehensive coverage.The test template covers server startup, tool listing, tool execution, and response validation. The 100ms delay after server start (line 557) is a reasonable approach for integration tests.
822-861: LGTM: Vanilla template properly handles tool results.The tool result handling with type definitions and subscription logic mirrors the React template approach appropriately.
1195-1258: LGTM: Scaffolding properly handles protocol and uses npm.The protocol parameter is correctly defaulted and propagated. Using npm for standalone projects (line 1252) is a good choice to avoid workspace-related issues.
169-171: No action required. Theconfig.protocolfield is fully supported by@mcp-apps-kit/core. Theprotocolfield is a documented property inGlobalConfigthat accepts"mcp"(default) or"openai"and is properly handled throughout the core package's server initialization and adapter system.
108-108: No changes needed—zod 4.x is the current release line as of January 2026, with 4.0.0 having been released in July 2025. The version specification^4.0.0is valid and will not cause installation failures.Likely an incorrect or invalid review comment.
packages/create-app/tests/unit/cli.test.ts (1)
71-88: LGTM: Comprehensive protocol parsing test coverage.The test cases properly cover default values, argument parsing (both long and short forms), and validation for the new protocol option.
packages/create-app/tests/integration/scaffold.test.ts (5)
124-140: LGTM: Test setup file verification is thorough.The test properly verifies that all three test configuration files are generated for the React template.
142-160: LGTM: Testing dependencies and scripts are properly verified.The test ensures that testing dependencies and npm scripts are correctly added to the generated project.
162-175: LGTM: Server export verification is appropriate.The test confirms that the generated server file exports the app instance, which is essential for the integration tests.
226-242: LGTM: Vanilla template test setup verification mirrors React.The test structure is consistent and provides equivalent coverage for the Vanilla template.
244-262: LGTM: Vanilla template testing dependencies are verified.The test ensures consistent testing infrastructure across both templates.
packages/create-app/src/cli.ts (8)
9-11: LGTM: Inquirer and figlet imports are appropriate.The specific named imports from
@inquirer/promptsand thefigletimport align with the dependency changes and CLI enhancements.
22-22: LGTM: Protocol field properly typed in CLIOptions.The protocol field is correctly defined as a required union type in CLIOptions, which is appropriate since the CLI always provides a default value.
38-59: LGTM: Project name validation refactored cleanly.The validation logic is simplified with the extracted
validateSimpleNamehelper, making it easier to maintain and understand.
68-105: LGTM: Protocol argument parsing follows established patterns.The protocol validation and parsing logic is consistent with the existing template option implementation, with proper error handling and defaults.
123-132: LGTM: parseArgs return statement is clean.The return statement properly constructs the CLIOptions object with all fields, including the new protocol field.
142-154: LGTM: ASCII art header enhances CLI experience.The
printHeaderfunction creates an attractive visual header using figlet and chalk, improving the user experience.
156-222: LGTM: Interactive prompts are well-structured with clear protocol options.The protocol selection prompt provides helpful descriptions and follows the consistent inquirer pattern. Error handling for cancelled prompts is appropriate.
247-287: LGTM: Protocol properly propagated and displayed.The protocol is correctly included in the options object and displayed to the user. The simplified npm command suggestions align with the "always use npm for standalone projects" approach.
| async sendMessage(content: { type: string; text: string }): Promise<void> { | ||
| const openai = this.getOpenAI(); | ||
| if (openai && typeof openai.sendMessage === "function") { | ||
| await (openai.sendMessage as (content: { type: string; text: string }) => Promise<void>)( | ||
| content | ||
| ); | ||
| if (openai && typeof openai.sendFollowUpMessage === "function") { | ||
| await (openai.sendFollowUpMessage as (opts: { prompt: string }) => Promise<void>)({ | ||
| prompt: content.text, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and examine the context around lines 514-521
fd "openai.ts" packages/ui/src/adaptersRepository: AndurilCode/mcp-apps-kit
Length of output: 102
🏁 Script executed:
# Look for references to sendFollowUpMessage in the codebase
rg "sendFollowUpMessage" --type ts -nRepository: AndurilCode/mcp-apps-kit
Length of output: 1166
🏁 Script executed:
# Check the file size and read the relevant section
wc -l packages/ui/src/adapters/openai.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 106
🏁 Script executed:
# Read the sendMessage method and surrounding context
sed -n '510,530p' packages/ui/src/adapters/openai.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 720
🏁 Script executed:
# Read the type definition to understand the actual API
cat -n packages/ui/src/types.ts | grep -A 2 "sendFollowUpMessage"Repository: AndurilCode/mcp-apps-kit
Length of output: 167
🏁 Script executed:
# Read the client implementation
sed -n '114,125p' packages/ui/src/client.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 330
🏁 Script executed:
# Check broader context for error handling and whether content.type should be used
sed -n '490,545p' packages/ui/src/adapters/openai.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 1844
🏁 Script executed:
# Check what the content parameter's type property should be used for
rg "sendMessage.*type" packages/ui/src -A 2 -B 2 --type tsRepository: AndurilCode/mcp-apps-kit
Length of output: 2281
🏁 Script executed:
# Check if there are AppError imports or error handling patterns in this file
head -50 packages/ui/src/adapters/openai.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 1634
🏁 Script executed:
# Check the adapter interface definition for type parameter
cat -n packages/ui/src/adapters/types.ts | grep -A 3 "sendMessage"Repository: AndurilCode/mcp-apps-kit
Length of output: 192
🏁 Script executed:
# Check if AppError is used in this file or if error handling pattern exists
grep -n "AppError\|ErrorCode\|Error\|throw" packages/ui/src/adapters/openai.ts | head -20Repository: AndurilCode/mcp-apps-kit
Length of output: 463
🏁 Script executed:
# Check if types are exported as type-only
grep -n "export type\|export interface" packages/ui/src/adapters/openai.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 112
🏁 Script executed:
# Check if AppError is imported in this file
grep -n "import.*AppError\|import.*ErrorCode" packages/ui/src/adapters/openai.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 50
🏁 Script executed:
# Compare error handling pattern - check if other adapters use AppError
grep -n "AppError\|ErrorCode" packages/ui/src/adapters/*.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 728
🏁 Script executed:
# Check the actual type definition in types.ts more carefully
sed -n '400,410p' packages/ui/src/types.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 281
🏁 Script executed:
# Check what UIError is and where it's imported from
cat -n packages/ui/src/errors.ts | head -30Repository: AndurilCode/mcp-apps-kit
Length of output: 1046
🏁 Script executed:
# Check if packages/ui is under packages/**/*.ts pattern that requires AppError or UIError
head -20 packages/ui/src/adapters/openai.ts | grep -i "mcp-apps-kit"Repository: AndurilCode/mcp-apps-kit
Length of output: 50
🏁 Script executed:
# Check if @mcp-apps-kit/core is available and what it exports
fd "package.json" packages/ui | xargs grep -l "@mcp-apps-kit/core" 2>/dev/null || echo "Not found in packages/ui"Repository: AndurilCode/mcp-apps-kit
Length of output: 92
🏁 Script executed:
# Check the root or core package
cat packages/core/package.json 2>/dev/null | grep -A 5 "\"name\""Repository: AndurilCode/mcp-apps-kit
Length of output: 274
🏁 Script executed:
# Check what's exported from core
fd "index.ts" packages/core/src | head -5 && grep -n "export.*Error\|export.*AppError" packages/core/src/*.ts 2>/dev/null | head -10Repository: AndurilCode/mcp-apps-kit
Length of output: 545
🏁 Script executed:
# Check if packages/ui is actually supposed to use AppError or if UIError is the pattern for this package
cat packages/ui/package.json | grep -A 10 "dependencies"Repository: AndurilCode/mcp-apps-kit
Length of output: 207
🏁 Script executed:
# Review the entire error handling in openai.ts
grep -n "throw\|Error\|async" packages/ui/src/adapters/openai.ts | head -30Repository: AndurilCode/mcp-apps-kit
Length of output: 1262
🏁 Script executed:
# Check the sendMessage implementation more carefully for potential issues
sed -n '514,525p' packages/ui/src/adapters/openai.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 465
🏁 Script executed:
# Check how content.type is used in other adapters
grep -A 5 "async sendMessage" packages/ui/src/adapters/mcp.ts | head -10Repository: AndurilCode/mcp-apps-kit
Length of output: 340
🏁 Script executed:
# Check the mock adapter
grep -A 10 "async sendMessage" packages/ui/src/adapters/mock.ts | head -15Repository: AndurilCode/mcp-apps-kit
Length of output: 421
🏁 Script executed:
# Check if content.type is used anywhere else in the file
grep -n "content.type" packages/ui/src/adapters/openai.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 50
The OpenAI API method signature for sendFollowUpMessage is correct, but the content.type parameter should be prefixed with an underscore since it's unused in this method.
Change content: { type: string; text: string } to content: { _type: string; text: string } to comply with the coding guideline: "Remove all unused variables or prefix them with underscore (_)".
Note: The mcp.ts adapter validates content.type, so if this parameter should be used for message type handling, add similar validation.
🤖 Prompt for AI Agents
In @packages/ui/src/adapters/openai.ts around lines 514 - 521, The sendMessage
method's parameter currently declares an unused property `type`; rename it to
`_type` in the signature (change content: { type: string; text: string } to
content: { _type: string; text: string }) so the unused variable is prefixed
with an underscore, and keep the rest of sendMessage (including the call to
openai.sendFollowUpMessage with content.text) unchanged; if message-type
validation is needed instead of ignoring it, mirror the validation logic from
the mcp.ts adapter for `_type` and implement appropriate handling rather than
simply renaming.
- Updated `@inquirer/prompts` from `^7.5.0` to `^7.8.6` in both `package.json` and `pnpm-lock.yaml`. - Upgraded `figlet` from `^1.8.0` to `^1.9.4` in both `package.json` and `pnpm-lock.yaml`. - Increased `@inquirer/testing` version from `^2.1.0` to `^2.1.47` in both files. These updates ensure compatibility with the latest features and improvements in the respective packages.
…penAIAdapter - Implemented a check in the `sendMessage` method to throw an error if the content type is not "text", ensuring that only supported message types are processed. - This change enhances the robustness of the OpenAIAdapter by preventing unexpected behavior when unsupported message types are sent.
Code Review: Protocol Option and CLI ImprovementsI've completed a thorough review of this PR. Overall, this is a solid enhancement that adds protocol selection to the CLI and makes several improvements. Here's my detailed feedback: ✅ Strengths1. Well-Structured Protocol Addition
2. Improved Developer Experience
3. Excellent Test Coverage
4. Testing Infrastructure
5. OpenAI Adapter Improvements
|
Code Review for PR #78This PR adds protocol selection to the CLI scaffolder and includes several quality improvements. All tests pass (186 UI + 39 create-app tests). ✅ Strengths
|
2 participants
Summary
--protocol mcpor--protocol openaiwhen scaffolding new projects, allowing them to target specific platformssendFollowUpMessage,openExternal,setWidgetState)Test plan
🤖 Generated with Claude Code