Fail dev command on port errors

[miles-kt-inkeep]

No description provided.

[vercel]

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

Project	Deployment	Preview	Comments	Updated (UTC)
agents-docs	Ready	Preview	Comment	Oct 29, 2025 0:47am
agents-manage-api	Ready	Preview	Comment	Oct 29, 2025 0:47am
agents-manage-ui	Ready	Preview	Comment	Oct 29, 2025 0:47am
agents-run-api	Ready	Preview	Comment	Oct 29, 2025 0:47am

[claude]

Claude finished @miles-kt-inkeep's task —— View job

---

Code Review: Fail dev command on port errors

Todo List:

• Read repository guidelines (CLAUDE.md)
• Examine all changed files to understand implementation
• Research existing port handling patterns
• Analyze system architecture impact
• Provide comprehensive code review feedback

---

Summary

This PR adds port conflict detection and graceful failure to the development setup process. The implementation adds strictPort: true to vite configurations and implements port conflict detection in the project creation utility.

Technical Analysis

Changes Overview:

1. Vite Configuration (agents-manage-api/vite.config.ts:14, agents-run-api/vite.config.ts:14): Added strictPort: true
2. Port Detection Logic (packages/create-agents/src/utils.ts:409-450): New error detection and handling
3. Changeset (.changeset/four-apples-stare.md): Documents the feature

Architecture & Design Quality

✅ Strong Points:

• Proper error boundaries: Implements graceful failure with clear user messaging
• User experience: Provides actionable error messages with specific port numbers
• Process cleanup: Handles background process termination properly with SIGTERM/SIGKILL fallback
• Consistent patterns: Uses existing color coding and messaging patterns from the codebase

⚠️ Areas for Consideration:

1. Error Detection Reliability (Medium Priority)
The string-based error detection approach may be fragile:

const errorLogRunApi = 'run-api-quickstart:dev: Error: Port';
const errorLogManageApi = 'manage-api-quickstart:dev: Error: Port';

Considerations:

• These strings depend on specific Vite/turbo output formats that could change
• No handling for different error message formats or localization
• Could miss port conflicts if the exact string doesn't match

Recommendation: Consider using more robust detection methods (regex patterns, multiple error indicators, or actual port availability checks).

2. Race Condition Potential (Low Priority)

await new Promise((resolve) => setTimeout(resolve, 5000)); // Fixed 5-second wait

Considerations:

• Fixed timeout may not be sufficient for slower systems
• No dynamic detection of when servers are actually ready vs. failed
• Could fail to detect errors that occur after the 5-second window

Recommendation: Consider implementing actual readiness checks (HTTP health checks, log parsing for "ready" messages) rather than fixed timeouts.

3. Process Management Robustness (Low Priority)
The process cleanup logic has some edge cases:

process.kill(-devProcess.pid, 'SIGTERM');
// ... wait 1 second ...
process.kill(-devProcess.pid, 'SIGKILL');

Considerations:

• No verification that the process group actually exists before killing
• Could leave orphaned processes if the spawn process fails
• Generic catch-all error handling might mask important issues

System Integration

✅ Well-Integrated:

• Follows established error handling patterns with p.cancel() and colored output
• Consistent with existing project setup workflow
• Proper changeset documentation for version management

⚠️ Integration Considerations:

1. Port Hardcoding vs. Configuration (Medium Priority)

const manageApiPort = '3002';
const runApiPort = '3003';

Considerations:

• Ports are hardcoded in multiple places (vite configs, utils.ts, etc.)
• No single source of truth for port configuration
• Makes it difficult to customize ports for different environments

Recommendation: Consider extracting port configuration to a shared configuration file or environment variables.

2. Error Message Consistency (Low Priority)
The error messages are clear but could be more consistent with the broader system's error handling patterns.

Customer Impact

✅ Positive Customer Impact:

• Better developer experience: Clear feedback when ports are in use
• Faster problem resolution: Specific error messages help users identify and fix port conflicts quickly
• Prevents confusing states: Stops setup process before creating incomplete/broken states

⚠️ Potential Edge Cases:

• Users with non-standard development setups might hit false positives/negatives
• No guidance on how to resolve port conflicts (e.g., killing existing processes, changing ports)

Testing Considerations

Missing Test Coverage:
This feature would benefit from unit tests covering:

• Port conflict detection logic
• Error message formatting
• Process cleanup functionality
• Edge cases (process spawn failures, timeout scenarios)

Recommendation: Add tests to packages/create-agents/src/__tests__/utils.test.ts following the existing Vitest patterns.

Security & Performance

✅ Security:

• Proper process cleanup prevents resource leaks
• No security vulnerabilities identified

✅ Performance:

• Minimal performance impact
• Efficient process management

Recommendations

High Priority:

1. Add comprehensive test coverage for the new port conflict detection logic

Medium Priority:

2. Improve error detection reliability by using more robust pattern matching or actual port availability checks
3. Centralize port configuration to avoid hardcoded values across multiple files

Low Priority:

4. Enhance user guidance by providing specific next steps for resolving port conflicts
5. Consider dynamic timeout logic instead of fixed 5-second waits

Overall Assessment

This is a well-implemented feature that significantly improves the developer experience during project setup. The code follows established patterns and handles edge cases reasonably well. The main areas for improvement are around test coverage and making the error detection more robust.

The implementation demonstrates good understanding of process management, error handling, and user experience principles. The changes are appropriately scoped and don't introduce unnecessary complexity.

Recommendation: ✅ Approve with suggestions for future improvements

---

[changeset-bot]

🦋 Changeset detected

Latest commit: eb3caea

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

This PR includes changesets to release 7 packages

Name	Type
@inkeep/create-agents	Patch
@inkeep/agents-manage-api	Patch
@inkeep/agents-run-api	Patch
@inkeep/agents-manage-ui	Patch
@inkeep/agents-cli	Patch
@inkeep/agents-core	Patch
@inkeep/agents-sdk	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

[github-actions]

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