Code Review

Code Review

Guidelines for reviewing code changes in the @dotcontext/cli project.

When to Use

• Reviewing a pull request or diff
• Validating code during the Review (R) or Validation (V) phases of the PREVC workflow
• Checking a new service, generator, or utility for adherence to project patterns
• Evaluating MCP gateway handler implementations

Project Standards

TypeScript Configuration

• Target: ES2020, Module: CommonJS
• Strict mode enabled ("strict": true)
• Declarations and source maps generated
• Path alias: @generators/agents/* maps to src/generators/agents/*

Code Organization Patterns

Service Layer (22 services in src/services/):

• Each service lives in its own directory under src/services/
• Exports through index.ts barrel files
• Constructor takes an options object or required config
• Methods are async where I/O is involved
• Example: FillService, WorkflowService, AIContextMCPServer

Generators (src/generators/):

• Separate generators for docs, agents, plans, skills
• Accept options objects, return typed result objects (e.g., SkillGeneratorResult)
• Use frontmatter serialization from src/types/scaffoldFrontmatter.ts

Utilities (src/utils/):

• Pure functions or thin wrappers: frontMatter.ts, contentSanitizer.ts, gitService.ts
• CLI UI abstraction: cliUI.ts with CLIInterface type
• i18n: i18n.ts with TranslateFn type and en/pt-BR locale support

MCP Gateway Handlers (src/services/mcp/gateway/):

• One file per gateway: explore.ts, context.ts, sync.ts, etc.
• Exports: handler function, params type, options type, action type
• Uses createJsonResponse/createErrorResponse/createTextResponse from response.ts
• Barrel re-export through index.ts and gatewayTools.ts

Review Checklist

Architecture

• New code follows the service/generator/util separation
• No direct file I/O in generators -- delegate to services or fs-extra
• MCP handlers do not use console.log (use process.stderr.write for debug output)
• repoPath is never hardcoded; always resolved through options or getRepoPath()

TypeScript Quality

• No any types (use unknown + type guards where needed)
• Exported interfaces/types for all public APIs
• Async functions return typed Promises, not implicit any
• Optional dependencies (tree-sitter) have graceful fallbacks

Error Handling

• MCP gateway handlers return createErrorResponse() rather than throwing
• CLI commands catch errors and display via chalk/ora with user-friendly messages
• File operations use fs-extra methods which handle missing directories

Naming Conventions

• Files: camelCase (fillService.ts, codebaseAnalyzer.ts)
• Classes: PascalCase (SkillGenerator, SemanticContextBuilder)
• Interfaces: PascalCase, often with I-free naming (SkillMetadata, not ISkillMetadata)
• Constants: UPPER_SNAKE_CASE (BUILT_IN_SKILLS, PREVC_PHASE_ORDER)
• MCP tools: kebab-case (workflow-init, workflow-status)

Testing

• New services have corresponding .test.ts files
• Tests use jest.mock() for external dependencies (AI providers, file system)
• Temp directories created with fs.mkdtemp() and cleaned up in afterEach
• Mock CLIInterface and TranslateFn for service tests (see fillService.test.ts)

Frontmatter

• New scaffold files use v2 format with scaffoldVersion: "2.0.0"
• status field is set correctly (unfilled for new, filled after content generation)
• Type-specific fields present (skillSlug for skills, agentType for agents, etc.)

i18n

• New user-facing strings added to both en and pt-BR in src/utils/i18n.ts
• Translation keys follow dot-notation: 'commands.fill.description'
• CLI output uses t() function, not hardcoded strings

Instructions

Reviewing a Service Change

1. Check the service's public interface -- does it follow the options-object constructor pattern?
2. Verify barrel exports in index.ts are updated
3. Check for proper error handling (try/catch with typed errors)
4. Ensure no side effects in constructors (async init should be a separate method)
5. Look for dependency injection points (e.g., AIContextMCPServer accepts contextBuilder for testing)

Reviewing an MCP Tool Change

1. Verify Zod schema has .describe() on every param with action prefix
2. Check that the handler switch covers all actions in the enum
3. Confirm responses use createJsonResponse/createErrorResponse consistently
4. Verify new types are re-exported through gatewayTools.ts
5. Check that repoPath uses the caching mechanism via getRepoPath()

Reviewing a Generator Change

1. Check output paths are constructed with path.join(repoPath, outputDir, ...)
2. Verify force flag is respected (skip existing files when false)
3. Ensure frontmatter is generated via createSkillFrontmatter/serializeFrontmatter from scaffoldFrontmatter.ts
4. Check that the result type includes all relevant counts (generated, skipped, etc.)

Reviewing a Workflow Change

1. Verify phase transitions follow PREVC order (P -> R -> E -> V -> C)
2. Check scale-dependent phase inclusion (QUICK: E+V only, SMALL: P+E+V, MEDIUM: P+R+E+V, LARGE: all)
3. Ensure gate checks are enforced unless autonomous or force is true
4. Verify workflow status file is written to .context/runtime/workflows/