mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-01-31 06:42:03 +00:00
refactor: reorganize spec regeneration routes and add unit tests
- Removed the old spec regeneration routes and replaced them with a new structure under the app-spec directory for better modularity. - Introduced unit tests for common functionalities in app-spec, covering state management and error handling. - Added documentation on route organization patterns to improve maintainability and clarity for future development.
This commit is contained in:
Cody Seibert
@@ -27,7 +27,6 @@ import { createGitRoutes } from "./routes/git.js";
|
||||
import { createSetupRoutes } from "./routes/setup.js";
|
||||
import { createSuggestionsRoutes } from "./routes/suggestions.js";
|
||||
import { createModelsRoutes } from "./routes/models.js";
|
||||
import { createSpecRegenerationRoutes } from "./routes/spec-regeneration.js";
|
||||
import { createRunningAgentsRoutes } from "./routes/running-agents.js";
|
||||
import { createWorkspaceRoutes } from "./routes/workspace.js";
|
||||
import { createTemplatesRoutes } from "./routes/templates.js";
|
||||
@@ -41,6 +40,7 @@ import { AgentService } from "./services/agent-service.js";
|
||||
import { FeatureLoader } from "./services/feature-loader.js";
|
||||
import { AutoModeService } from "./services/auto-mode-service.js";
|
||||
import { getTerminalService } from "./services/terminal-service.js";
|
||||
import { createSpecRegenerationRoutes } from "./routes/app-spec/index.js";
|
||||
|
||||
// Load environment variables
|
||||
dotenv.config();
|
||||
|
||||
@@ -1,8 +0,0 @@
|
||||
/**
|
||||
* Spec Regeneration routes - HTTP API for AI-powered spec generation
|
||||
*
|
||||
* This file re-exports from the app-spec directory to maintain backward compatibility.
|
||||
* The actual implementation has been split into smaller modules in ./app-spec/
|
||||
*/
|
||||
|
||||
export { createSpecRegenerationRoutes } from "./app-spec/index.js";
|
||||
103
apps/server/tests/unit/routes/app-spec/common.test.ts
Normal file
103
apps/server/tests/unit/routes/app-spec/common.test.ts
Normal file
@@ -0,0 +1,103 @@
|
||||
import { describe, it, expect, beforeEach } from "vitest";
|
||||
import {
|
||||
setRunningState,
|
||||
getErrorMessage,
|
||||
isRunning,
|
||||
currentAbortController,
|
||||
} from "@/routes/app-spec/common.js";
|
||||
|
||||
describe("app-spec/common.ts", () => {
|
||||
beforeEach(() => {
|
||||
// Reset state before each test
|
||||
setRunningState(false, null);
|
||||
});
|
||||
|
||||
describe("setRunningState", () => {
|
||||
it("should set isRunning to true when running is true", () => {
|
||||
setRunningState(true);
|
||||
expect(isRunning).toBe(true);
|
||||
});
|
||||
|
||||
it("should set isRunning to false when running is false", () => {
|
||||
setRunningState(true);
|
||||
setRunningState(false);
|
||||
expect(isRunning).toBe(false);
|
||||
});
|
||||
|
||||
it("should set currentAbortController when provided", () => {
|
||||
const controller = new AbortController();
|
||||
setRunningState(true, controller);
|
||||
expect(currentAbortController).toBe(controller);
|
||||
});
|
||||
|
||||
it("should set currentAbortController to null when not provided", () => {
|
||||
const controller = new AbortController();
|
||||
setRunningState(true, controller);
|
||||
setRunningState(false);
|
||||
expect(currentAbortController).toBe(null);
|
||||
});
|
||||
|
||||
it("should set currentAbortController to null when explicitly passed null", () => {
|
||||
const controller = new AbortController();
|
||||
setRunningState(true, controller);
|
||||
setRunningState(true, null);
|
||||
expect(currentAbortController).toBe(null);
|
||||
});
|
||||
|
||||
it("should update state multiple times correctly", () => {
|
||||
const controller1 = new AbortController();
|
||||
const controller2 = new AbortController();
|
||||
|
||||
setRunningState(true, controller1);
|
||||
expect(isRunning).toBe(true);
|
||||
expect(currentAbortController).toBe(controller1);
|
||||
|
||||
setRunningState(true, controller2);
|
||||
expect(isRunning).toBe(true);
|
||||
expect(currentAbortController).toBe(controller2);
|
||||
|
||||
setRunningState(false, null);
|
||||
expect(isRunning).toBe(false);
|
||||
expect(currentAbortController).toBe(null);
|
||||
});
|
||||
});
|
||||
|
||||
describe("getErrorMessage", () => {
|
||||
it("should return message from Error instance", () => {
|
||||
const error = new Error("Test error message");
|
||||
expect(getErrorMessage(error)).toBe("Test error message");
|
||||
});
|
||||
|
||||
it("should return 'Unknown error' for non-Error objects", () => {
|
||||
expect(getErrorMessage("string error")).toBe("Unknown error");
|
||||
expect(getErrorMessage(123)).toBe("Unknown error");
|
||||
expect(getErrorMessage(null)).toBe("Unknown error");
|
||||
expect(getErrorMessage(undefined)).toBe("Unknown error");
|
||||
expect(getErrorMessage({})).toBe("Unknown error");
|
||||
expect(getErrorMessage([])).toBe("Unknown error");
|
||||
});
|
||||
|
||||
it("should return message from Error with empty message", () => {
|
||||
const error = new Error("");
|
||||
expect(getErrorMessage(error)).toBe("");
|
||||
});
|
||||
|
||||
it("should handle Error objects with custom properties", () => {
|
||||
const error = new Error("Base message");
|
||||
(error as any).customProp = "custom value";
|
||||
expect(getErrorMessage(error)).toBe("Base message");
|
||||
});
|
||||
|
||||
it("should handle Error objects created with different constructors", () => {
|
||||
class CustomError extends Error {
|
||||
constructor(message: string) {
|
||||
super(message);
|
||||
this.name = "CustomError";
|
||||
}
|
||||
}
|
||||
|
||||
const customError = new CustomError("Custom error message");
|
||||
expect(getErrorMessage(customError)).toBe("Custom error message");
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,244 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
|
||||
describe("app-spec/parse-and-create-features.ts - JSON extraction", () => {
|
||||
// Test the JSON extraction regex pattern used in parseAndCreateFeatures
|
||||
const jsonExtractionPattern = /\{[\s\S]*"features"[\s\S]*\}/;
|
||||
|
||||
describe("JSON extraction regex", () => {
|
||||
it("should extract JSON with features array", () => {
|
||||
const content = `Here is the response:
|
||||
{
|
||||
"features": [
|
||||
{
|
||||
"id": "feature-1",
|
||||
"title": "Test Feature",
|
||||
"description": "A test feature",
|
||||
"priority": 1,
|
||||
"complexity": "simple",
|
||||
"dependencies": []
|
||||
}
|
||||
]
|
||||
}`;
|
||||
|
||||
const match = content.match(jsonExtractionPattern);
|
||||
expect(match).not.toBeNull();
|
||||
expect(match![0]).toContain('"features"');
|
||||
expect(match![0]).toContain('"id": "feature-1"');
|
||||
});
|
||||
|
||||
it("should extract JSON with multiple features", () => {
|
||||
const content = `Some text before
|
||||
{
|
||||
"features": [
|
||||
{
|
||||
"id": "feature-1",
|
||||
"title": "Feature 1"
|
||||
},
|
||||
{
|
||||
"id": "feature-2",
|
||||
"title": "Feature 2"
|
||||
}
|
||||
]
|
||||
}
|
||||
Some text after`;
|
||||
|
||||
const match = content.match(jsonExtractionPattern);
|
||||
expect(match).not.toBeNull();
|
||||
expect(match![0]).toContain('"features"');
|
||||
expect(match![0]).toContain('"feature-1"');
|
||||
expect(match![0]).toContain('"feature-2"');
|
||||
});
|
||||
|
||||
it("should extract JSON with nested objects and arrays", () => {
|
||||
const content = `Response:
|
||||
{
|
||||
"features": [
|
||||
{
|
||||
"id": "feature-1",
|
||||
"dependencies": ["dep-1", "dep-2"],
|
||||
"metadata": {
|
||||
"tags": ["tag1", "tag2"]
|
||||
}
|
||||
}
|
||||
]
|
||||
}`;
|
||||
|
||||
const match = content.match(jsonExtractionPattern);
|
||||
expect(match).not.toBeNull();
|
||||
expect(match![0]).toContain('"dependencies"');
|
||||
expect(match![0]).toContain('"dep-1"');
|
||||
});
|
||||
|
||||
it("should handle JSON with whitespace and newlines", () => {
|
||||
const content = `Text before
|
||||
{
|
||||
"features": [
|
||||
{
|
||||
"id": "feature-1",
|
||||
"title": "Feature",
|
||||
"description": "A feature\nwith newlines"
|
||||
}
|
||||
]
|
||||
}
|
||||
Text after`;
|
||||
|
||||
const match = content.match(jsonExtractionPattern);
|
||||
expect(match).not.toBeNull();
|
||||
expect(match![0]).toContain('"features"');
|
||||
});
|
||||
|
||||
it("should extract JSON when features array is empty", () => {
|
||||
const content = `Response:
|
||||
{
|
||||
"features": []
|
||||
}`;
|
||||
|
||||
const match = content.match(jsonExtractionPattern);
|
||||
expect(match).not.toBeNull();
|
||||
expect(match![0]).toContain('"features"');
|
||||
expect(match![0]).toContain("[]");
|
||||
});
|
||||
|
||||
it("should not match content without features key", () => {
|
||||
const content = `{
|
||||
"otherKey": "value"
|
||||
}`;
|
||||
|
||||
const match = content.match(jsonExtractionPattern);
|
||||
expect(match).toBeNull();
|
||||
});
|
||||
|
||||
it("should not match content without JSON structure", () => {
|
||||
const content = "Just plain text with features mentioned";
|
||||
const match = content.match(jsonExtractionPattern);
|
||||
expect(match).toBeNull();
|
||||
});
|
||||
|
||||
it("should extract JSON when features key appears multiple times", () => {
|
||||
const content = `Before:
|
||||
{
|
||||
"features": [
|
||||
{
|
||||
"id": "feature-1",
|
||||
"title": "Feature"
|
||||
}
|
||||
]
|
||||
}
|
||||
After: The word "features" appears again`;
|
||||
|
||||
const match = content.match(jsonExtractionPattern);
|
||||
expect(match).not.toBeNull();
|
||||
// Should match from first { to last }
|
||||
expect(match![0]).toContain('"features"');
|
||||
});
|
||||
|
||||
it("should handle JSON with escaped quotes", () => {
|
||||
const content = `{
|
||||
"features": [
|
||||
{
|
||||
"id": "feature-1",
|
||||
"description": "A feature with \\"quotes\\""
|
||||
}
|
||||
]
|
||||
}`;
|
||||
|
||||
const match = content.match(jsonExtractionPattern);
|
||||
expect(match).not.toBeNull();
|
||||
expect(match![0]).toContain('"features"');
|
||||
});
|
||||
|
||||
it("should extract JSON with complex nested structure", () => {
|
||||
const content = `Response:
|
||||
{
|
||||
"features": [
|
||||
{
|
||||
"id": "feature-1",
|
||||
"dependencies": [
|
||||
{
|
||||
"id": "dep-1",
|
||||
"type": "required"
|
||||
}
|
||||
],
|
||||
"metadata": {
|
||||
"tags": ["tag1"],
|
||||
"notes": "Some notes"
|
||||
}
|
||||
}
|
||||
],
|
||||
"metadata": {
|
||||
"version": "1.0"
|
||||
}
|
||||
}`;
|
||||
|
||||
const match = content.match(jsonExtractionPattern);
|
||||
expect(match).not.toBeNull();
|
||||
expect(match![0]).toContain('"features"');
|
||||
expect(match![0]).toContain('"metadata"');
|
||||
});
|
||||
});
|
||||
|
||||
describe("JSON parsing validation", () => {
|
||||
it("should parse valid feature JSON structure", () => {
|
||||
const validJson = `{
|
||||
"features": [
|
||||
{
|
||||
"id": "feature-1",
|
||||
"title": "Test Feature",
|
||||
"description": "A test feature",
|
||||
"priority": 1,
|
||||
"complexity": "simple",
|
||||
"dependencies": []
|
||||
}
|
||||
]
|
||||
}`;
|
||||
|
||||
const parsed = JSON.parse(validJson);
|
||||
expect(parsed.features).toBeDefined();
|
||||
expect(Array.isArray(parsed.features)).toBe(true);
|
||||
expect(parsed.features.length).toBe(1);
|
||||
expect(parsed.features[0].id).toBe("feature-1");
|
||||
expect(parsed.features[0].title).toBe("Test Feature");
|
||||
});
|
||||
|
||||
it("should handle features with optional fields", () => {
|
||||
const jsonWithOptionalFields = `{
|
||||
"features": [
|
||||
{
|
||||
"id": "feature-1",
|
||||
"title": "Feature",
|
||||
"priority": 2,
|
||||
"complexity": "moderate"
|
||||
}
|
||||
]
|
||||
}`;
|
||||
|
||||
const parsed = JSON.parse(jsonWithOptionalFields);
|
||||
expect(parsed.features[0].id).toBe("feature-1");
|
||||
expect(parsed.features[0].priority).toBe(2);
|
||||
// description and dependencies are optional
|
||||
expect(parsed.features[0].description).toBeUndefined();
|
||||
expect(parsed.features[0].dependencies).toBeUndefined();
|
||||
});
|
||||
|
||||
it("should handle features with dependencies", () => {
|
||||
const jsonWithDeps = `{
|
||||
"features": [
|
||||
{
|
||||
"id": "feature-1",
|
||||
"title": "Feature 1",
|
||||
"dependencies": []
|
||||
},
|
||||
{
|
||||
"id": "feature-2",
|
||||
"title": "Feature 2",
|
||||
"dependencies": ["feature-1"]
|
||||
}
|
||||
]
|
||||
}`;
|
||||
|
||||
const parsed = JSON.parse(jsonWithDeps);
|
||||
expect(parsed.features[0].dependencies).toEqual([]);
|
||||
expect(parsed.features[1].dependencies).toEqual(["feature-1"]);
|
||||
});
|
||||
});
|
||||
});
|
||||
582
docs/server/route-organization.md
Normal file
582
docs/server/route-organization.md
Normal file
@@ -0,0 +1,582 @@
|
||||
# Route Organization Pattern
|
||||
|
||||
This document describes the pattern used for organizing Express routes into modular, maintainable file structures. This pattern is exemplified by the `app-spec` route module and should be applied to other route modules for consistency and maintainability.
|
||||
|
||||
---
|
||||
|
||||
## Table of Contents
|
||||
|
||||
1. [Overview](#overview)
|
||||
2. [Directory Structure](#directory-structure)
|
||||
3. [File Organization Principles](#file-organization-principles)
|
||||
4. [File Types and Their Roles](#file-types-and-their-roles)
|
||||
5. [Implementation Guidelines](#implementation-guidelines)
|
||||
6. [Example: app-spec Module](#example-app-spec-module)
|
||||
7. [Migration Guide](#migration-guide)
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
The route organization pattern separates concerns into:
|
||||
|
||||
- **Route handlers** - Thin HTTP request/response handlers in `routes/` subdirectory
|
||||
- **Business logic** - Extracted into standalone function files
|
||||
- **Shared utilities** - Common functions and state in `common.ts`
|
||||
- **Route registration** - Centralized in `index.ts`
|
||||
|
||||
This pattern improves:
|
||||
|
||||
- **Maintainability** - Clear separation of concerns
|
||||
- **Testability** - Functions can be tested independently
|
||||
- **Reusability** - Business logic can be reused across routes
|
||||
- **Readability** - Smaller, focused files are easier to understand
|
||||
|
||||
---
|
||||
|
||||
## Directory Structure
|
||||
|
||||
```
|
||||
routes/
|
||||
└── {module-name}/
|
||||
├── index.ts # Route registration & export
|
||||
├── common.ts # Shared utilities & state
|
||||
├── {business-function}.ts # Extracted business logic functions
|
||||
└── routes/
|
||||
├── {endpoint-name}.ts # Individual route handlers
|
||||
└── ...
|
||||
```
|
||||
|
||||
### Example Structure
|
||||
|
||||
```
|
||||
routes/
|
||||
└── app-spec/
|
||||
├── index.ts # createSpecRegenerationRoutes()
|
||||
├── common.ts # Shared state, logging utilities
|
||||
├── generate-spec.ts # generateSpec() function
|
||||
├── generate-features-from-spec.ts # generateFeaturesFromSpec() function
|
||||
├── parse-and-create-features.ts # parseAndCreateFeatures() function
|
||||
└── routes/
|
||||
├── create.ts # POST /create handler
|
||||
├── generate.ts # POST /generate handler
|
||||
├── generate-features.ts # POST /generate-features handler
|
||||
├── status.ts # GET /status handler
|
||||
└── stop.ts # POST /stop handler
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## File Organization Principles
|
||||
|
||||
### 1. **Single Responsibility**
|
||||
|
||||
Each file should have one clear purpose:
|
||||
|
||||
- Route handlers handle HTTP concerns (request/response, validation)
|
||||
- Business logic files contain domain-specific operations
|
||||
- Common files contain shared utilities and state
|
||||
|
||||
### 2. **Separation of Concerns**
|
||||
|
||||
- **HTTP Layer** (`routes/*.ts`) - Request parsing, response formatting, status codes
|
||||
- **Business Logic** (`*.ts` in root) - Core functionality, domain operations
|
||||
- **Shared State** (`common.ts`) - Module-level state, cross-cutting utilities
|
||||
|
||||
### 3. **File Size Management**
|
||||
|
||||
- Extract functions when files exceed ~150-200 lines
|
||||
- Extract when a function is reusable across multiple routes
|
||||
- Extract when a function has complex logic that deserves its own file
|
||||
|
||||
### 4. **Naming Conventions**
|
||||
|
||||
- Route handlers: `{verb}-{resource}.ts` or `{action}.ts` (e.g., `create.ts`, `status.ts`)
|
||||
- Business logic: `{action}-{noun}.ts` or `{verb}-{noun}.ts` (e.g., `generate-spec.ts`)
|
||||
- Common utilities: Always `common.ts`
|
||||
|
||||
---
|
||||
|
||||
## File Types and Their Roles
|
||||
|
||||
### `index.ts` - Route Registration
|
||||
|
||||
**Purpose**: Central export point that creates and configures the Express router.
|
||||
|
||||
**Responsibilities**:
|
||||
|
||||
- Import route handler factories
|
||||
- Create Express Router instance
|
||||
- Register all routes
|
||||
- Export router creation function
|
||||
|
||||
**Pattern**:
|
||||
|
||||
```typescript
|
||||
import { Router } from "express";
|
||||
import type { EventEmitter } from "../../lib/events.js";
|
||||
import { createCreateHandler } from "./routes/create.js";
|
||||
import { createGenerateHandler } from "./routes/generate.js";
|
||||
|
||||
export function create{Module}Routes(events: EventEmitter): Router {
|
||||
const router = Router();
|
||||
|
||||
router.post("/create", createCreateHandler(events));
|
||||
router.get("/status", createStatusHandler());
|
||||
|
||||
return router;
|
||||
}
|
||||
```
|
||||
|
||||
**Key Points**:
|
||||
|
||||
- Function name: `create{Module}Routes`
|
||||
- Accepts dependencies (e.g., `EventEmitter`) as parameters
|
||||
- Returns configured Router instance
|
||||
- Route handlers are factory functions that accept dependencies
|
||||
|
||||
---
|
||||
|
||||
### `common.ts` - Shared Utilities & State
|
||||
|
||||
**Purpose**: Central location for shared state, utilities, and helper functions used across multiple route handlers and business logic files.
|
||||
|
||||
**Common Contents**:
|
||||
|
||||
- Module-level state (e.g., `isRunning`, `currentAbortController`)
|
||||
- State management functions (e.g., `setRunningState()`)
|
||||
- Logging utilities (e.g., `logAuthStatus()`, `logError()`)
|
||||
- Error handling utilities (e.g., `getErrorMessage()`)
|
||||
- Shared constants
|
||||
- Shared types/interfaces
|
||||
|
||||
**Pattern**:
|
||||
|
||||
```typescript
|
||||
import { createLogger } from "../../lib/logger.js";
|
||||
|
||||
const logger = createLogger("{ModuleName}");
|
||||
|
||||
// Shared state
|
||||
export let isRunning = false;
|
||||
export let currentAbortController: AbortController | null = null;
|
||||
|
||||
// State management
|
||||
export function setRunningState(
|
||||
running: boolean,
|
||||
controller: AbortController | null = null
|
||||
): void {
|
||||
isRunning = running;
|
||||
currentAbortController = controller;
|
||||
}
|
||||
|
||||
// Utility functions
|
||||
export function logError(error: unknown, context: string): void {
|
||||
logger.error(`❌ ${context}:`, error);
|
||||
}
|
||||
|
||||
export function getErrorMessage(error: unknown): string {
|
||||
return error instanceof Error ? error.message : "Unknown error";
|
||||
}
|
||||
```
|
||||
|
||||
**Key Points**:
|
||||
|
||||
- Export shared state as `let` variables (mutable state)
|
||||
- Provide setter functions for state management
|
||||
- Keep utilities focused and reusable
|
||||
- Use consistent logging patterns
|
||||
|
||||
---
|
||||
|
||||
### `routes/{endpoint-name}.ts` - Route Handlers
|
||||
|
||||
**Purpose**: Thin HTTP request/response handlers that validate input, call business logic, and format responses.
|
||||
|
||||
**Responsibilities**:
|
||||
|
||||
- Parse and validate request parameters
|
||||
- Check preconditions (e.g., `isRunning` state)
|
||||
- Call business logic functions
|
||||
- Handle errors and format responses
|
||||
- Manage background tasks (if applicable)
|
||||
|
||||
**Pattern**:
|
||||
|
||||
```typescript
|
||||
import type { Request, Response } from "express";
|
||||
import type { EventEmitter } from "../../../lib/events.js";
|
||||
import { createLogger } from "../../../lib/logger.js";
|
||||
import {
|
||||
isRunning,
|
||||
setRunningState,
|
||||
logError,
|
||||
getErrorMessage,
|
||||
} from "../common.js";
|
||||
import { businessLogicFunction } from "../business-logic.js";
|
||||
|
||||
const logger = createLogger("{ModuleName}");
|
||||
|
||||
export function create{Action}Handler(events: EventEmitter) {
|
||||
return async (req: Request, res: Response): Promise<void> => {
|
||||
logger.info("========== /{endpoint} endpoint called ==========");
|
||||
|
||||
try {
|
||||
// 1. Parse and validate input
|
||||
const { param1, param2 } = req.body as { param1: string; param2?: number };
|
||||
|
||||
if (!param1) {
|
||||
res.status(400).json({ success: false, error: "param1 required" });
|
||||
return;
|
||||
}
|
||||
|
||||
// 2. Check preconditions
|
||||
if (isRunning) {
|
||||
res.json({ success: false, error: "Operation already running" });
|
||||
return;
|
||||
}
|
||||
|
||||
// 3. Set up state
|
||||
const abortController = new AbortController();
|
||||
setRunningState(true, abortController);
|
||||
|
||||
// 4. Call business logic (background if async)
|
||||
businessLogicFunction(param1, param2, events, abortController)
|
||||
.catch((error) => {
|
||||
logError(error, "Operation failed");
|
||||
events.emit("module:event", { type: "error", error: getErrorMessage(error) });
|
||||
})
|
||||
.finally(() => {
|
||||
setRunningState(false, null);
|
||||
});
|
||||
|
||||
// 5. Return immediate response
|
||||
res.json({ success: true });
|
||||
} catch (error) {
|
||||
logger.error("❌ Route handler exception:", error);
|
||||
res.status(500).json({ success: false, error: getErrorMessage(error) });
|
||||
}
|
||||
};
|
||||
}
|
||||
```
|
||||
|
||||
**Key Points**:
|
||||
|
||||
- Factory function pattern: `create{Action}Handler(dependencies)`
|
||||
- Returns async Express handler function
|
||||
- Validate input early
|
||||
- Use shared utilities from `common.ts`
|
||||
- Handle errors consistently
|
||||
- For background tasks, return success immediately and handle completion asynchronously
|
||||
|
||||
---
|
||||
|
||||
### `{business-function}.ts` - Business Logic Files
|
||||
|
||||
**Purpose**: Standalone files containing complex business logic functions that can be reused across routes or extracted to reduce file size.
|
||||
|
||||
**When to Extract**:
|
||||
|
||||
- Function exceeds ~100-150 lines
|
||||
- Function is called from multiple route handlers
|
||||
- Function has complex logic that deserves its own file
|
||||
- Function can be tested independently
|
||||
|
||||
**Pattern**:
|
||||
|
||||
```typescript
|
||||
/**
|
||||
* {Brief description of what this function does}
|
||||
*/
|
||||
|
||||
import { query, type Options } from "@anthropic-ai/claude-agent-sdk";
|
||||
import type { EventEmitter } from "../../lib/events.js";
|
||||
import { createLogger } from "../../lib/logger.js";
|
||||
import { logAuthStatus } from "./common.js";
|
||||
import { anotherBusinessFunction } from "./another-business-function.js";
|
||||
|
||||
const logger = createLogger("{ModuleName}");
|
||||
|
||||
export async function businessLogicFunction(
|
||||
param1: string,
|
||||
param2: number,
|
||||
events: EventEmitter,
|
||||
abortController: AbortController
|
||||
): Promise<void> {
|
||||
logger.debug("========== businessLogicFunction() started ==========");
|
||||
|
||||
try {
|
||||
// Business logic here
|
||||
// ...
|
||||
|
||||
// Can call other business logic functions
|
||||
await anotherBusinessFunction(param1, events, abortController);
|
||||
|
||||
logger.debug("========== businessLogicFunction() completed ==========");
|
||||
} catch (error) {
|
||||
logger.error("❌ businessLogicFunction() failed:", error);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Key Points**:
|
||||
|
||||
- Export named functions (not default exports)
|
||||
- Include JSDoc comment at top
|
||||
- Import shared utilities from `common.ts`
|
||||
- Use consistent logging patterns
|
||||
- Can import and call other business logic functions
|
||||
- Handle errors and re-throw or emit events as appropriate
|
||||
|
||||
---
|
||||
|
||||
## Implementation Guidelines
|
||||
|
||||
### Step 1: Create Directory Structure
|
||||
|
||||
```bash
|
||||
mkdir -p routes/{module-name}/routes
|
||||
```
|
||||
|
||||
### Step 2: Create `common.ts`
|
||||
|
||||
Start with shared state and utilities:
|
||||
|
||||
- Module-level state variables
|
||||
- State management functions
|
||||
- Logging utilities
|
||||
- Error handling utilities
|
||||
|
||||
### Step 3: Extract Business Logic
|
||||
|
||||
Identify large functions or reusable logic:
|
||||
|
||||
- Functions > 150 lines → extract to separate file
|
||||
- Functions used by multiple routes → extract to separate file
|
||||
- Complex operations → extract to separate file
|
||||
|
||||
### Step 4: Create Route Handlers
|
||||
|
||||
For each endpoint:
|
||||
|
||||
- Create `routes/{endpoint-name}.ts`
|
||||
- Implement factory function pattern
|
||||
- Keep handlers thin (validation + call business logic)
|
||||
- Use utilities from `common.ts`
|
||||
|
||||
### Step 5: Create `index.ts`
|
||||
|
||||
- Import all route handler factories
|
||||
- Create router and register routes
|
||||
- Export router creation function
|
||||
|
||||
### Step 6: Register Module
|
||||
|
||||
In main routes file:
|
||||
|
||||
```typescript
|
||||
import { create{Module}Routes } from "./{module-name}/index.js";
|
||||
|
||||
app.use("/api/{module-name}", create{Module}Routes(events));
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Example: app-spec Module
|
||||
|
||||
The `app-spec` module demonstrates this pattern:
|
||||
|
||||
### File Breakdown
|
||||
|
||||
**`index.ts`** (24 lines)
|
||||
|
||||
- Creates router
|
||||
- Registers 5 endpoints
|
||||
- Exports `createSpecRegenerationRoutes()`
|
||||
|
||||
**`common.ts`** (74 lines)
|
||||
|
||||
- Shared state: `isRunning`, `currentAbortController`
|
||||
- State management: `setRunningState()`
|
||||
- Utilities: `logAuthStatus()`, `logError()`, `getErrorMessage()`
|
||||
|
||||
**`generate-spec.ts`** (204 lines)
|
||||
|
||||
- Extracted business logic for spec generation
|
||||
- Handles SDK calls, streaming, file I/O
|
||||
- Called by both `create.ts` and `generate.ts` routes
|
||||
|
||||
**`generate-features-from-spec.ts`** (155 lines)
|
||||
|
||||
- Extracted business logic for feature generation
|
||||
- Handles SDK calls and streaming
|
||||
- Calls `parseAndCreateFeatures()` for final step
|
||||
|
||||
**`parse-and-create-features.ts`** (84 lines)
|
||||
|
||||
- Extracted parsing and file creation logic
|
||||
- Called by `generate-features-from-spec.ts`
|
||||
|
||||
**`routes/create.ts`** (96 lines)
|
||||
|
||||
- Thin handler for POST /create
|
||||
- Validates input, checks state, calls `generateSpec()`
|
||||
|
||||
**`routes/generate.ts`** (99 lines)
|
||||
|
||||
- Thin handler for POST /generate
|
||||
- Similar to `create.ts` but different input parameter
|
||||
|
||||
**`routes/generate-features.ts`** (71 lines)
|
||||
|
||||
- Thin handler for POST /generate-features
|
||||
- Calls `generateFeaturesFromSpec()`
|
||||
|
||||
**`routes/status.ts`** (17 lines)
|
||||
|
||||
- Simple handler for GET /status
|
||||
- Returns current state
|
||||
|
||||
**`routes/stop.ts`** (25 lines)
|
||||
|
||||
- Simple handler for POST /stop
|
||||
- Aborts current operation
|
||||
|
||||
### Key Observations
|
||||
|
||||
1. **Route handlers are thin** - Most are 70-100 lines, focused on HTTP concerns
|
||||
2. **Business logic is extracted** - Complex operations in separate files
|
||||
3. **Shared utilities centralized** - Common functions in `common.ts`
|
||||
4. **Reusability** - `generateSpec()` used by both `create.ts` and `generate.ts`
|
||||
5. **Clear separation** - HTTP layer vs business logic vs shared utilities
|
||||
|
||||
---
|
||||
|
||||
## Migration Guide
|
||||
|
||||
### Migrating an Existing Route Module
|
||||
|
||||
1. **Analyze current structure**
|
||||
|
||||
- Identify all endpoints
|
||||
- Identify shared state/utilities
|
||||
- Identify large functions (>150 lines)
|
||||
|
||||
2. **Create directory structure**
|
||||
|
||||
```bash
|
||||
mkdir -p routes/{module-name}/routes
|
||||
```
|
||||
|
||||
3. **Extract common utilities**
|
||||
|
||||
- Move shared state to `common.ts`
|
||||
- Move utility functions to `common.ts`
|
||||
- Update imports in existing files
|
||||
|
||||
4. **Extract business logic**
|
||||
|
||||
- Identify functions to extract
|
||||
- Create `{function-name}.ts` files
|
||||
- Move logic, update imports
|
||||
|
||||
5. **Create route handlers**
|
||||
|
||||
- Create `routes/{endpoint-name}.ts` for each endpoint
|
||||
- Move HTTP handling logic
|
||||
- Keep handlers thin
|
||||
|
||||
6. **Create index.ts**
|
||||
|
||||
- Import route handlers
|
||||
- Register routes
|
||||
- Export router creation function
|
||||
|
||||
7. **Update main routes file**
|
||||
|
||||
- Import from new `index.ts`
|
||||
- Update route registration
|
||||
|
||||
8. **Test**
|
||||
- Verify all endpoints work
|
||||
- Check error handling
|
||||
- Verify shared state management
|
||||
|
||||
### Example Migration
|
||||
|
||||
**Before** (monolithic `routes.ts`):
|
||||
|
||||
```typescript
|
||||
// routes.ts - 500+ lines
|
||||
router.post("/create", async (req, res) => {
|
||||
// 200 lines of logic
|
||||
});
|
||||
|
||||
router.post("/generate", async (req, res) => {
|
||||
// 200 lines of similar logic
|
||||
});
|
||||
```
|
||||
|
||||
**After** (organized structure):
|
||||
|
||||
```typescript
|
||||
// routes/app-spec/index.ts
|
||||
export function createSpecRegenerationRoutes(events) {
|
||||
const router = Router();
|
||||
router.post("/create", createCreateHandler(events));
|
||||
router.post("/generate", createGenerateHandler(events));
|
||||
return router;
|
||||
}
|
||||
|
||||
// routes/app-spec/routes/create.ts - 96 lines
|
||||
export function createCreateHandler(events) {
|
||||
return async (req, res) => {
|
||||
// Thin handler, calls generateSpec()
|
||||
};
|
||||
}
|
||||
|
||||
// routes/app-spec/generate-spec.ts - 204 lines
|
||||
export async function generateSpec(...) {
|
||||
// Business logic extracted here
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Best Practices
|
||||
|
||||
### ✅ Do
|
||||
|
||||
- Keep route handlers thin (< 150 lines)
|
||||
- Extract complex business logic to separate files
|
||||
- Centralize shared utilities in `common.ts`
|
||||
- Use factory function pattern for route handlers
|
||||
- Export named functions (not default exports)
|
||||
- Use consistent logging patterns
|
||||
- Handle errors consistently
|
||||
- Document complex functions with JSDoc
|
||||
|
||||
### ❌ Don't
|
||||
|
||||
- Put business logic directly in route handlers
|
||||
- Duplicate utility functions across files
|
||||
- Create files with only one small function (< 20 lines)
|
||||
- Mix HTTP concerns with business logic
|
||||
- Use default exports for route handlers
|
||||
- Create deeply nested directory structures
|
||||
- Put route handlers in root of module directory
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
The route organization pattern provides:
|
||||
|
||||
1. **Clear structure** - Easy to find and understand code
|
||||
2. **Separation of concerns** - HTTP, business logic, and utilities separated
|
||||
3. **Reusability** - Business logic can be shared across routes
|
||||
4. **Maintainability** - Smaller, focused files are easier to maintain
|
||||
5. **Testability** - Functions can be tested independently
|
||||
|
||||
Apply this pattern to all route modules for consistency and improved code quality.
|
||||
Reference in New Issue
Block a user