code-refactoring

Code Refactoring Patterns

Overview

This skill provides patterns and checklists for refactoring code in the Orient monorepo. Use this when:

• Extracting shared logic from duplicate implementations
• Migrating services between src/ and packages/
• Unifying database access patterns
• Consolidating tool definitions

Pattern 1: Service Extraction

When to use: Two or more services share identical logic (>50 lines or complex logic).

Checklist

1. Identify Shared Code

   • Find duplicate implementations in the codebase
   • Document the differences (usually import paths and configuration)
   • Verify the logic is semantically identical

2. Design the Shared Interface

   // Define configuration options
   export interface ServiceConfig {
     option1?: string;
     option2?: number;
   }
   
   // Define the executor/​adapter pattern
   export type ServiceExecutor = (input: Input, context?: Context) => Promise<Result>;
   

3. Create the Shared Service

   • Place in src/​services/ or appropriate package
   • Make all platform-specific parts configurable
   • Add comprehensive JSDoc comments

4. Update Consumers

   • Import from the new shared service
   • Pass platform-specific configuration
   • Remove duplicate inline implementations

5. Write Tests

   • Unit tests for the shared service
   • Integration tests for each consumer
   • Before/​after comparison tests if needed

Example: Tool-Calling Service

The toolCallingService.ts extracts the common tool-calling loop from AgentService and WhatsAppAgentService:

// Shared service with configurable behavior
export async function executeToolLoop(
  anthropic: Anthropic,
  messages: MessageParam[],
  tools: Tool[],
  executor: ToolExecutor, // Platform-specific
  config: ToolCallingConfig,
  context?: unknown // Platform-specific context
): Promise<ToolCallingResult>;

Consumers pass their own tool executor while benefiting from shared guardrails.

Pattern 2: Package Migration

When to use: Moving code from src/ to packages/ for better modularity.

Checklist

1. Create Package Structure

   packages/[package-name]/
   ├── package.json
   ├── tsconfig.json
   ├── vitest.config.ts
   ├── README.md
   ├── src/
   │   ├── index.ts
   │   └── [implementation files]
   └── __tests__/
   

2. Define Package Dependencies

   • Use "@orientbot/​core": "workspace:*" for internal deps
   • Keep dependencies minimal
   • Avoid circular dependencies

3. Update tsconfig.json Path Mappings

   {
     "compilerOptions": {
       "paths": {
         "@orientbot/[package]": ["./​packages/[package]/​src/​index.ts"]
       }
     }
   }
   

4. Incremental Migration

   • Start with types-only export if the migration is complex
   • Add deprecation notices to old files
   • Update consumers one at a time
   • Run tests after each consumer update

5. Verify Package Builds

   pnpm --filter @orientbot/[package] build
   pnpm --filter @orientbot/[package] test
   

Example: @orientbot/​database-services

// packages/​database-services/​src/​index.ts
export { MessageDatabase, StoredMessage, StoredGroup } from './​messageDatabase.js';
export { SlackDatabase } from './​slackDatabase.js';
export { SchedulerDatabase } from './​schedulerDatabase.js';
export { WebhookDatabase } from './​webhookDatabase.js';
export * from './​types/​index.js';

Pattern 3: Database Access Unification

When to use: Multiple database access patterns coexist (raw SQL, BaseDatabase, Drizzle).

Checklist

1. Audit Current Usage

   grep -r "BaseDatabase" src/
   grep -r "getDatabase" src/
   grep -r "executeRawSql" src/
   

2. Create Comparison Tests

   • Write tests that verify old and new implementations produce same results
   • Include edge cases and error scenarios

3. Create Drizzle Helpers

   // In @orientbot/​database
   export async function executeRaw<T>(sql: string, params?: unknown[]): Promise<T[]>;
   export async function executeRawOne<T>(sql: string, params?: unknown[]): Promise<T | null>;
   

4. Migrate Services One at a Time

   • Start with services that already use Drizzle partially
   • Update imports to use @orientbot/​database
   • Run comparison tests after each migration

5. Deprecate Old Patterns

   • Add @deprecated JSDoc to old implementations
   • Set timeline for removal

Pattern 4: Tool Definition Consolidation

When to use: Same tools defined in multiple places.

Checklist

1. Create Tool Definitions Directory

   src/​tools/​definitions/
   ├── index.ts
   ├── jira.ts
   └── whatsapp.ts
   

2. Extract Common Definitions

   // src/​tools/​definitions/​jira.ts
   export const JIRA_TOOL_DEFINITIONS: Anthropic.Tool[] = [
     { name: 'get_all_issues', ... },
     // ...
   ];
   

3. Create Platform-Specific Getters

   export function getSlackJiraTools(): Anthropic.Tool[] {
     return [...JIRA_TOOL_DEFINITIONS];
   }
   
   export function getWhatsAppJiraTools(): Anthropic.Tool[] {
     return [...JIRA_TOOL_DEFINITIONS, ...EXTENDED_TOOLS];
   }
   

4. Update Consumers

   • Import from definitions instead of defining inline
   • Test that all tools are still available

Common Pitfalls

1. Breaking Import Paths

• Always update tsconfig.json path mappings
• Use .js extensions for ESM compatibility
• Build dependent packages before consumers

2. Missing Type Exports

• Export types explicitly: export type { TypeName }
• Re-export from index.ts for convenience

3. Circular Dependencies

• Keep packages loosely coupled
• Use dependency injection patterns
• Types can often break circular deps

4. Test Coverage Gaps

• Write tests BEFORE refactoring
• Use comparison tests for migrations
• Run tests after each incremental change

Pattern 5: Import Path Migration

When to use: Migrating imports from old paths to new package exports, or consolidating scattered imports.

Pre-Migration Audit

Before changing any imports, audit the current state:

# Find all imports of a specific module
grep -r "from '.*oldModule'" packages/ src/ --include="*.ts"

# Count imports by pattern
grep -r "from '@orientbot/​old" packages/ --include="*.ts" | wc -l

# Find files importing from dist (should be zero)
grep -r "from '.*dist/" packages/ src/ --include="*.ts"

Migration Checklist

1. Build the target package first

   pnpm --filter @orientbot/​new-package build
   ls packages/​new-package/​dist/  # Verify dist exists
   

2. Add workspace dependency to consumer's package.json

   {
     "dependencies": {
       "@orientbot/​new-package": "workspace:*"
     }
   }
   

3. Update imports one file at a time

   // Before
   import { Service } from '../​../​old/​path/​service.js';
   
   // After
   import { Service } from '@orientbot/​new-package';
   

4. Verify after each file

   pnpm --filter @orientbot/​consumer-package build
   pnpm --filter @orientbot/​consumer-package test
   

5. Check for runtime issues

   • Build passes but runtime fails = missing export or ESM/​CJS issue
   • See esm-cjs-interop and package-exports skills for solutions

Import Path Decision Tree

Avoiding Dist Imports

Never import directly from another package's dist:

// ❌ Wrong - brittle, breaks on rebuild
import { X } from '../​../​../​packages/​core/​dist/​index.js';
import { X } from '@orientbot/​core/​dist/​something.js';

// ✅ Correct - uses package exports
import { X } from '@orientbot/​core';

Add this test to prevent regressions:

// tests/​no-dist-imports.test.ts
import { describe, it, expect } from 'vitest';
import { execSync } from 'child_process';

describe('Import hygiene', () => {
  it('should not import from dist directories', () => {
    const result = execSync("grep -r \"from '.*dist/\" packages/ src/ --include='*.ts' || true", {
      encoding: 'utf-8',
    });
    expect(result.trim()).toBe('');
  });
});

Bulk Migration Script

For large migrations, use a script:

#!/​bin/​bash
# migrate-imports.sh

OLD_IMPORT="from '../​../​old/​service"
NEW_IMPORT="from '@orientbot/​new-package"

find packages/ src/ -name "*.ts" -exec \
  sed -i '' "s|${OLD_IMPORT}|${NEW_IMPORT}|g" {} \;

echo "Updated files:"
git diff --name-only

Post-Migration Verification

# Full rebuild
pnpm turbo build --force

# Run all tests
pnpm test

# Type check
pnpm turbo typecheck

# Check for leftover old imports
grep -r "old/​import/​path" packages/ src/ --include="*.ts"

Package Dependency Graph

@orientbot/​core (foundation)
    ├── @orientbot/​database (Drizzle ORM)
    ├── @orientbot/​database-services (MessageDB, SlackDB, etc.)
    ├── @orientbot/​integrations (JIRA, Google)
    ├── @orientbot/​mcp-tools (tool implementations)
    ├── @orientbot/​bot-whatsapp
    ├── @orientbot/​bot-slack
    ├── @orientbot/​api-gateway
    └── @orientbot/​dashboard

Migration Status Tracking

Use this template to track migration progress:

| Component          | Status   | Package                      | Notes                           |
| ------------------ | -------- | ---------------------------- | ------------------------------- |
| jiraService        | partial  | @orientbot/​integrations      | Types exported, service pending |
| messageDatabase    | complete | @orientbot/​database-services |                                 |
| toolCallingService | complete | src/​services/                | Shared between agents           |