Code Patterns

Code Patterns

Best practices distilled from implementation lessons. These patterns prevent common bugs in file I/O, data persistence, and validation.

Core Principle

LEARN FROM MISTAKES - DON'T REPEAT THEM

Each pattern here was extracted from a real bug or issue. Apply these patterns proactively to prevent the same mistakes.

When to Use

Always use for: File-based state, shared resources, data persistence, validation layers, concurrent access

Especially when: Multiple processes/hooks access same file, persisting JSON data, creating factory functions, implementing save/load logic

The Patterns

1. File Locking (references/file-io.md)

Advisory file locking for shared state access.

Problem: Multiple processes writing to same file causes data corruption or race conditions.

Solution: Use .lock file pattern with timeout and stale lock detection.

⚠️ MUST READ when: Implementing file-based state accessed by multiple hooks/processes

2. Atomic Writes (references/data-persistence.md)

Safe file persistence that survives crashes.

Problem: writeFileSync() mid-write crash corrupts file.

Solution: Write to .tmp, rename to final path (atomic on POSIX), handle Windows with backup pattern.

⚠️ MUST READ when: Persisting any JSON/data file that must survive unexpected termination

3. Schema Validation (references/data-validation.md)

Validate before every persist operation.

Problem: Factory functions create invalid data that persists and causes downstream issues.

Solution: Validate at creation AND before every write. Never trust "it was validated earlier."

⚠️ MUST READ when: Creating factory functions, implementing create/update operations, building data pipelines

Quick Reference

Shared file access → file-io.md (add locking)
JSON persistence → data-persistence.md (atomic writes)
Factory function → data-validation.md (validate output)

Prevention Checklist

Before implementing file I/O:

• [ ] Will multiple processes access this file?
• [ ] What happens if write crashes mid-operation?
• [ ] Is data validated before every write?
• [ ] Are stale locks handled (dead process detection)?

Origin

These patterns were extracted from hook infrastructure implementation review:

• Race condition in concurrent file writes → file-io.md
• File corruption risk on crash → data-persistence.md
• Invalid data persisting without validation → data-validation.md

IMPORTANT Task Planning Notes

• Always plan and break many small todo tasks
• Always add a final review todo task to review the works done at the end to find any fix or enhancement needed

Atomic Writes for Data Persistence

Problem Statement

Direct file writes are not atomic:

• Process crash mid-write leaves partial/corrupt file
• Power loss during write causes data loss
• writeFileSync() provides no atomicity guarantees

Real Example: JSON state file corruption when process crashed during write, leaving truncated JSON that couldn't be parsed on next load.

Solution: Temp File + Rename Pattern

Write to temporary file first, then rename to final destination. rename() is atomic on POSIX systems.

POSIX Implementation

const fs = require('fs');

/**
 * Atomic JSON write - writes to temp file then renames
 */
function atomicWriteJSON(filePath, data) {
  const tmpPath = filePath + '.tmp';
  const content = JSON.stringify(data, null, 2);

  // Write to temp file first
  fs.writeFileSync(tmpPath, content, 'utf8');

  // Atomic rename to final path
  fs.renameSync(tmpPath, filePath);
}

Windows-Safe Implementation

Windows rename may fail if target exists. Use backup pattern:

const fs = require('fs');

/**
 * Atomic JSON write - Windows safe
 */
function atomicWriteJSON(filePath, data) {
  const tmpPath = filePath + '.tmp';
  const bakPath = filePath + '.bak';
  const content = JSON.stringify(data, null, 2);

  // Step 1: Write to temp file
  fs.writeFileSync(tmpPath, content, 'utf8');

  // Step 2: Backup original if exists
  try {
    if (fs.existsSync(filePath)) {
      fs.renameSync(filePath, bakPath);
    }
  } catch { /* no original file */ }

  // Step 3: Rename temp to final (atomic)
  fs.renameSync(tmpPath, filePath);

  // Step 4: Clean up backup
  try {
    fs.unlinkSync(bakPath);
  } catch { /* no backup or cleanup failed */ }
}

With File Locking

For shared state, combine with file locking:

function saveState(state) {
  return withLock(() => {
    atomicWriteJSON(STATE_FILE, state);
  });
}

Key Principles

1. Never write directly to final path - Always use temp file 2. Rename is atomic - One operation, no partial states 3. Handle Windows differences - Backup pattern for cross-platform 4. Clean up on startup - Remove leftover .tmp/.bak files 5. Validate before write - Ensure data is valid JSON

Recovery Pattern

On startup, check for incomplete writes:

function recoverFromCrash(filePath) {
  const tmpPath = filePath + '.tmp';
  const bakPath = filePath + '.bak';

  // Case 1: Temp file exists but final doesn't
  // (crashed after temp write, before rename)
  if (fs.existsSync(tmpPath) && !fs.existsSync(filePath)) {
    try {
      const data = JSON.parse(fs.readFileSync(tmpPath, 'utf8'));
      fs.renameSync(tmpPath, filePath);
      console.log('Recovered from temp file');
    } catch {
      fs.unlinkSync(tmpPath); // Corrupt temp, discard
    }
  }

  // Case 2: Backup exists but final doesn't
  // (crashed during Windows rename sequence)
  if (fs.existsSync(bakPath) && !fs.existsSync(filePath)) {
    fs.renameSync(bakPath, filePath);
    console.log('Recovered from backup');
  }

  // Case 3: Both temp and final exist - temp is stale
  if (fs.existsSync(tmpPath) && fs.existsSync(filePath)) {
    fs.unlinkSync(tmpPath);
  }

  // Case 4: Both backup and final exist - backup is stale
  if (fs.existsSync(bakPath) && fs.existsSync(filePath)) {
    fs.unlinkSync(bakPath);
  }
}

Anti-Patterns

// WRONG: Direct write - not atomic
fs.writeFileSync(filePath, JSON.stringify(data));

// WRONG: No error handling on temp write
const tmp = filePath + '.tmp';
fs.writeFileSync(tmp, data); // Could fail
fs.renameSync(tmp, filePath); // Leaves corrupt .tmp

// WRONG: Using copy instead of rename
fs.copyFileSync(tmpPath, filePath); // Not atomic!
fs.unlinkSync(tmpPath);

Verification

Test crash resilience:

// Simulate crash during write
function testCrashResilience() {
  const testFile = 'test-state.json';

  // Write initial state
  atomicWriteJSON(testFile, { count: 1 });

  // Simulate crash: create .tmp but don't complete rename
  fs.writeFileSync(testFile + '.tmp', JSON.stringify({ count: 2 }));

  // Recovery should restore from backup or use original
  recoverFromCrash(testFile);

  const state = JSON.parse(fs.readFileSync(testFile, 'utf8'));
  console.log('Recovered state:', state);
}

Performance Note

Atomic writes have slight overhead (extra I/O for rename). For high-frequency writes:

• Batch updates before persisting
• Use write-ahead log for critical data
• Consider SQLite for structured data (has built-in atomicity)

Schema Validation Before Persist

Problem Statement

Invalid data can persist and cause downstream issues:

• Factory functions create invalid objects that get saved
• Validation at read time doesn't prevent corrupt writes
• "It was validated earlier" is never reliable

Real Example: A factory function didn't validate its output. Invalid objects with empty required fields persisted and caused injection failures later.

Solution: Validate at Every Boundary

Validate data: 1. At creation (factory functions) 2. Before every write 3. At every trust boundary (API responses, message consumption)

Factory Function Pattern

/**
 * Create entity with validation
 */
function createEntity(input, options = {}) {
  const entity = {
    id: input.id || generateId(),
    name: input.name || '',
    status: input.status || 'pending',
    createdAt: input.createdAt || new Date().toISOString(),
    // ... other fields with defaults
  };

  // Always validate unless explicitly skipped
  if (!options.skipValidation) {
    const errors = validateEntity(entity);
    if (errors.length > 0) {
      throw new Error(`Invalid entity: ${errors.join(', ')}`);
    }
  }

  return entity;
}

/**
 * Validate entity schema
 */
function validateEntity(entity) {
  const errors = [];

  if (!entity) {
    return ['Entity is null or undefined'];
  }

  // Required field checks
  if (!entity.id || typeof entity.id !== 'string') {
    errors.push('id is required and must be a string');
  }

  if (!entity.name || entity.name.length < 1) {
    errors.push('name is required and cannot be empty');
  }

  // Format checks
  if (entity.createdAt && !isValidISODate(entity.createdAt)) {
    errors.push('createdAt must be a valid ISO date string');
  }

  // Range checks
  if (entity.count !== undefined && (entity.count < 0 || entity.count > 1000)) {
    errors.push('count must be between 0 and 1000');
  }

  return errors;
}

function isValidISODate(str) {
  const date = new Date(str);
  return date instanceof Date && !isNaN(date) && date.toISOString() === str;
}

Save with Validation Pattern

/**
 * Save entities with pre-save validation
 */
function saveEntities(entities) {
  // Validate all before saving any
  const allErrors = [];
  entities.forEach((entity, index) => {
    const errors = validateEntity(entity);
    if (errors.length > 0) {
      allErrors.push(`Entity ${index}: ${errors.join(', ')}`);
    }
  });

  if (allErrors.length > 0) {
    throw new Error(`Validation failed:\n${allErrors.join('\n')}`);
  }

  // All valid, proceed with save
  atomicWriteJSON(ENTITIES_FILE, entities);
}

Update with Validation Pattern

/**
 * Update entity with validation
 */
function updateEntity(id, updates) {
  return withLock(() => {
    const entities = loadEntities();
    const entity = entities.find(e => e.id === id);

    if (!entity) {
      throw new Error(`Entity not found: ${id}`);
    }

    // Apply updates
    const updated = { ...entity, ...updates, updatedAt: new Date().toISOString() };

    // Validate updated entity
    const errors = validateEntity(updated);
    if (errors.length > 0) {
      throw new Error(`Invalid update: ${errors.join(', ')}`);
    }

    // Replace in array
    const index = entities.indexOf(entity);
    entities[index] = updated;

    saveEntities(entities);
    return updated;
  });
}

Key Principles

1. Validate at creation - Factory functions validate output 2. Validate before every write - Never trust "validated earlier" 3. Fail fast - Reject invalid data immediately 4. Collect all errors - Don't stop at first error 5. Bounded values - Prevent overflow with min/max checks

Bounds Checking Pattern

Prevent integer overflow and unbounded growth:

const MAX_COUNT = 1000;
const MAX_ITEMS = 100;

/**
 * Increment count with bounds checking
 */
function incrementCount(current, amount = 1) {
  return Math.min((current || 0) + amount, MAX_COUNT);
}

/**
 * Add item with size limit
 */
function addItem(array, item, maxItems = MAX_ITEMS) {
  if (!array) array = [];
  array.push(item);
  return array.slice(-maxItems); // Keep only last N items
}

Anti-Patterns

// WRONG: No validation in factory
function createEntity(input) {
  return { ...defaultEntity, ...input }; // Invalid input passes through
}

// WRONG: Validation only at read
function loadEntities() {
  const entities = JSON.parse(fs.readFileSync(FILE));
  return entities.filter(e => validateEntity(e).length === 0);
  // Invalid entities already in file!
}

// WRONG: Trust "validated earlier"
async function processAndSave(rawInput) {
  const validated = validateInput(rawInput); // Validated here
  const processed = await slowProcess(validated); // But what if this changes it?
  saveEntity(processed); // No validation before save!
}

// WRONG: Unbounded growth
delta.helpful_count = delta.helpful_count + 1; // Can overflow
delta.source_events.push(event); // Array grows forever

Verification

// Test validation catches invalid data
function testValidation() {
  // Should throw
  try {
    createEntity({ name: '' }); // Empty name
    console.error('FAIL: Should have thrown');
  } catch (e) {
    console.log('PASS: Caught invalid entity');
  }

  // Should pass
  try {
    const entity = createEntity({ name: 'Valid' });
    console.log('PASS: Valid entity created');
  } catch (e) {
    console.error('FAIL: Should not have thrown');
  }
}

Integration with Locking and Atomic Writes

Complete pattern combining all three:

function saveEntitySafely(entity) {
  // 1. Validate before anything
  const errors = validateEntity(entity);
  if (errors.length > 0) {
    throw new Error(`Invalid entity: ${errors.join(', ')}`);
  }

  // 2. Lock for read-modify-write
  return withLock(() => {
    const entities = loadEntities();

    // 3. Check for duplicates or conflicts
    const existing = entities.find(e => e.id === entity.id);
    if (existing) {
      const index = entities.indexOf(existing);
      entities[index] = entity;
    } else {
      entities.push(entity);
    }

    // 4. Atomic write
    atomicWriteJSON(ENTITIES_FILE, entities);
    return entity;
  });
}

File Locking for Shared State

Problem Statement

When multiple processes/hooks may access the same file simultaneously:

• Concurrent reads during write cause partial/corrupt data
• Concurrent writes cause data loss (last write wins)
• No coordination leads to race conditions

Real Example: Multiple hooks accessing the same JSON state file. Without locking, simultaneous updates corrupt data.

Solution: Advisory File Locking

Use a .lock file pattern with timeout and stale lock detection.

Implementation Pattern

const fs = require('fs');

const LOCK_FILE = 'state.lock';
const LOCK_TIMEOUT_MS = 5000;
const LOCK_RETRY_DELAY_MS = 50;

/**
 * Check if a process is still alive
 */
function isProcessAlive(pid) {
  try {
    process.kill(pid, 0);
    return true;
  } catch {
    return false;
  }
}

/**
 * Synchronous sleep (for retry loops)
 */
function sleepSync(ms) {
  const end = Date.now() + ms;
  while (Date.now() < end) { /* busy wait */ }
}

/**
 * Acquire file lock with timeout
 */
function acquireLock() {
  const deadline = Date.now() + LOCK_TIMEOUT_MS;

  while (Date.now() < deadline) {
    try {
      // O_EXCL fails if file exists - atomic lock creation
      fs.writeFileSync(LOCK_FILE, process.pid.toString(), { flag: 'wx' });
      return true;
    } catch (err) {
      if (err.code === 'EEXIST') {
        // Check if lock is stale (owning process dead)
        try {
          const pid = parseInt(fs.readFileSync(LOCK_FILE, 'utf8'), 10);
          if (!isProcessAlive(pid)) {
            fs.unlinkSync(LOCK_FILE);
            continue; // Retry immediately
          }
        } catch { /* ignore read errors */ }
        sleepSync(LOCK_RETRY_DELAY_MS);
      } else {
        throw err;
      }
    }
  }
  return false;
}

/**
 * Release file lock
 */
function releaseLock() {
  try {
    fs.unlinkSync(LOCK_FILE);
  } catch { /* ignore if already released */ }
}

/**
 * Execute function with file lock
 */
function withLock(fn) {
  if (!acquireLock()) {
    console.error('[Lock] Timeout - skipping operation');
    return null;
  }
  try {
    return fn();
  } finally {
    releaseLock();
  }
}

Usage

// Wrap read-modify-write operations
function updateState(updater) {
  return withLock(() => {
    const state = loadState();
    const newState = updater(state);
    saveState(newState);
    return newState;
  });
}

// Example: Increment counter safely
updateState(state => ({
  ...state,
  counter: (state.counter || 0) + 1
}));

Key Principles

1. Always acquire lock BEFORE read - Not just before write 2. Handle stale locks - Dead process detection prevents permanent lockout 3. Use timeout - Don't wait forever 4. Wrap entire read-modify-write - Not just the write 5. Release in finally block - Ensure release even on error

Anti-Patterns

// WRONG: Lock only protects write
const state = loadState(); // Race: another process can modify between load and save
acquireLock();
saveState(state);
releaseLock();

// WRONG: No stale lock detection
while (fs.existsSync(LOCK_FILE)) {
  sleep(50); // Can wait forever if process crashed
}

// WRONG: No timeout
while (!tryLock()) {
  sleep(50); // Can block indefinitely
}

Windows Considerations

Windows file locking differs from POSIX:

• fs.renameSync may fail if target exists
• Use backup pattern: write .tmp → rename original to .bak → rename .tmp to final
• Check for leftover .tmp/.bak files on startup

Verification

Test concurrent access:

# Run multiple instances simultaneously
for i in {1..10}; do
  node hook.cjs &
done
wait

# Verify no data corruption
node -e "console.log(JSON.parse(require('fs').readFileSync('state.json')))"