feat: final test coverage gaps for #54

package.json

@@ -17,15 +17,15 @@
     "pretest": "rm -rf test-db test-logs",
     "test": "echo '\n⚠️  WARNING: Running full test suite crashes Claude Code instances!\n\n✅ Safe commands (run these from Claude Code):\n   npm run test:core\n   npm run test:handlers\n   npm run test:repositories\n   npm run test:adapters\n   npm run test:implementations\n   npm run test:services\n   npm run test:cli\n   npm run test:integration\n\n❌ Full suite: Use npm run test:all (only in local terminal/CI)\n' && exit 1",
     "test:all": "npm run test:core && npm run test:handlers && npm run test:services && npm run test:repositories && npm run test:adapters && npm run test:implementations && npm run test:cli && npm run test:scheduling && npm run test:checkpoints && npm run test:error-scenarios && npm run test:integration",
-    "test:services": "NODE_OPTIONS='--max-old-space-size=2048' vitest run tests/unit/services/task-manager.test.ts tests/unit/services/recovery-manager.test.ts tests/unit/services/autoscaling-manager.test.ts --no-file-parallelism",
+    "test:services": "NODE_OPTIONS='--max-old-space-size=2048' vitest run tests/unit/services/task-manager.test.ts tests/unit/services/recovery-manager.test.ts tests/unit/services/autoscaling-manager.test.ts tests/unit/services/process-connector.test.ts --no-file-parallelism",
     "test:full": "npm run test:all && npm run test:worker-handler",
     "test:unit": "NODE_OPTIONS='--max-old-space-size=2048' vitest run --no-file-parallelism",
     "test:core": "NODE_OPTIONS='--max-old-space-size=2048' vitest run tests/unit/core --no-file-parallelism",
     "test:handlers": "NODE_OPTIONS='--max-old-space-size=2048' vitest run tests/unit/services/handlers/dependency-handler.test.ts tests/unit/services/handlers/query-handler.test.ts tests/unit/services/handlers/schedule-handler.test.ts tests/unit/services/handlers/checkpoint-handler.test.ts tests/unit/services/handlers/persistence-handler.test.ts tests/unit/services/handlers/queue-handler.test.ts tests/unit/services/handlers/output-handler.test.ts --no-file-parallelism",
     "test:worker-handler": "NODE_OPTIONS='--max-old-space-size=2048' vitest run tests/unit/services/handlers/worker-handler.test.ts --no-file-parallelism --testTimeout=60000",
-    "test:repositories": "NODE_OPTIONS='--max-old-space-size=2048' vitest run tests/unit/implementations/dependency-repository.test.ts tests/unit/implementations/task-repository.test.ts tests/unit/implementations/database.test.ts tests/unit/implementations/checkpoint-repository.test.ts --no-file-parallelism",
+    "test:repositories": "NODE_OPTIONS='--max-old-space-size=2048' vitest run tests/unit/implementations/dependency-repository.test.ts tests/unit/implementations/task-repository.test.ts tests/unit/implementations/database.test.ts tests/unit/implementations/checkpoint-repository.test.ts tests/unit/implementations/output-repository.test.ts --no-file-parallelism",
     "test:adapters": "NODE_OPTIONS='--max-old-space-size=2048' vitest run tests/unit/adapters --no-file-parallelism",
-    "test:implementations": "NODE_OPTIONS='--max-old-space-size=2048' vitest run tests/unit/implementations --exclude='**/dependency-repository.test.ts' --exclude='**/task-repository.test.ts' --exclude='**/database.test.ts' --no-file-parallelism",
+    "test:implementations": "NODE_OPTIONS='--max-old-space-size=2048' vitest run tests/unit/implementations --exclude='**/dependency-repository.test.ts' --exclude='**/task-repository.test.ts' --exclude='**/database.test.ts' --exclude='**/checkpoint-repository.test.ts' --exclude='**/output-repository.test.ts' --no-file-parallelism",
     "test:scheduling": "NODE_OPTIONS='--max-old-space-size=2048' vitest run tests/unit/services/schedule-manager.test.ts tests/unit/services/schedule-executor.test.ts tests/unit/services/handlers/schedule-handler.test.ts --no-file-parallelism",
     "test:checkpoints": "NODE_OPTIONS='--max-old-space-size=2048' vitest run tests/unit/implementations/checkpoint-repository.test.ts tests/unit/services/handlers/checkpoint-handler.test.ts --no-file-parallelism",
     "test:cli": "NODE_OPTIONS='--max-old-space-size=2048' vitest run tests/unit/cli.test.ts tests/unit/cli-init.test.ts tests/unit/retry-functionality.test.ts --no-file-parallelism",

src/utils/git-state.ts

@@ -55,10 +55,9 @@ export async function captureGitState(workingDirectory: string): Promise<Result<
       const statusResult = await execFileAsync('git', ['status', '--porcelain'], execOpts);
       if (statusResult.stdout.trim()) {
         dirtyFiles = statusResult.stdout
-          .trim()
           .split('\n')
-          .map((line) => line.substring(3).trim()) // Remove status prefix (e.g., " M ", "?? ")
-          .filter((file) => file.length > 0);
+          .filter((line) => line.length > 0)
+          .map((line) => line.substring(3).trim()); // Remove status prefix (e.g., " M ", "?? ")
       }
     } catch {
       // Status failed - continue with empty dirty files

tests/unit/implementations/output-repository.test.ts

@@ -0,0 +1,167 @@
+/**
+ * Unit tests for SQLiteOutputRepository
+ *
+ * ARCHITECTURE: Tests output persistence with real Database + SQLite
+ * Pattern: Mirrors task-repository.test.ts — real DB, no mocks
+ * Note: task_output has FK to tasks — must insert task rows first
+ */
+
+import fs from 'node:fs';
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import { type Configuration, ConfigurationSchema } from '../../../src/core/configuration.js';
+import { TaskId } from '../../../src/core/domain.js';
+import { Database } from '../../../src/implementations/database.js';
+import { SQLiteOutputRepository } from '../../../src/implementations/output-repository.js';
+import { SQLiteTaskRepository } from '../../../src/implementations/task-repository.js';
+import { createTestTask } from '../../fixtures/test-data.js';
+
+describe('SQLiteOutputRepository', () => {
+  let database: Database;
+  let repo: SQLiteOutputRepository;
+  let taskRepo: SQLiteTaskRepository;
+  const taskId = TaskId('test-task-1');
+
+  beforeEach(async () => {
+    database = new Database(':memory:');
+    const config: Configuration = ConfigurationSchema.parse({
+      fileStorageThresholdBytes: 1024, // 1KB threshold for tests
+    });
+    repo = new SQLiteOutputRepository(config, database);
+    taskRepo = new SQLiteTaskRepository(database);
+
+    // Insert a task row to satisfy FK constraint
+    await taskRepo.save(createTestTask({ id: taskId }));
+  });
+
+  afterEach(() => {
+    database.close();
+    // Clean up ./output/ directory created by file-backed storage with :memory: DB
+    fs.rmSync('output', { recursive: true, force: true });
+  });
+
+  describe('save and get', () => {
+    it('should save small output to DB and retrieve it', async () => {
+      const output = {
+        taskId,
+        stdout: ['hello world'],
+        stderr: [],
+        totalSize: 11,
+      };
+
+      const saveResult = await repo.save(taskId, output);
+      expect(saveResult.ok).toBe(true);
+
+      const getResult = await repo.get(taskId);
+      expect(getResult.ok).toBe(true);
+      if (!getResult.ok) return;
+      expect(getResult.value).not.toBeNull();
+      expect(getResult.value!.stdout).toEqual(['hello world']);
+      expect(getResult.value!.stderr).toEqual([]);
+    });
+
+    it('should save large output to file (above fileStorageThreshold)', async () => {
+      const largeData = 'x'.repeat(2048); // 2KB > 1KB threshold
+      const output = {
+        taskId,
+        stdout: [largeData],
+        stderr: [],
+        totalSize: 2048,
+      };
+
+      const saveResult = await repo.save(taskId, output);
+      expect(saveResult.ok).toBe(true);
+
+      const getResult = await repo.get(taskId);
+      expect(getResult.ok).toBe(true);
+      if (!getResult.ok) return;
+      expect(getResult.value).not.toBeNull();
+      expect(getResult.value!.stdout).toEqual([largeData]);
+    });
+
+    it('should return null for missing task', async () => {
+      const result = await repo.get(TaskId('nonexistent'));
+      expect(result.ok).toBe(true);
+      if (!result.ok) return;
+      expect(result.value).toBeNull();
+    });
+  });
+
+  describe('append', () => {
+    it('should append to existing output', async () => {
+      const output = {
+        taskId,
+        stdout: ['line1'],
+        stderr: [],
+        totalSize: 5,
+      };
+      await repo.save(taskId, output);
+
+      const appendResult = await repo.append(taskId, 'stdout', 'line2');
+      expect(appendResult.ok).toBe(true);
+
+      const getResult = await repo.get(taskId);
+      expect(getResult.ok).toBe(true);
+      if (!getResult.ok) return;
+      expect(getResult.value!.stdout).toEqual(['line1', 'line2']);
+    });
+
+    it('should create new output if none exists', async () => {
+      const newTaskId = TaskId('new-task');
+      // Insert task row for FK
+      await taskRepo.save(createTestTask({ id: newTaskId }));
+
+      const appendResult = await repo.append(newTaskId, 'stderr', 'error msg');
+      expect(appendResult.ok).toBe(true);
+
+      const getResult = await repo.get(newTaskId);
+      expect(getResult.ok).toBe(true);
+      if (!getResult.ok) return;
+      expect(getResult.value!.stderr).toEqual(['error msg']);
+      expect(getResult.value!.stdout).toEqual([]);
+    });
+  });
+
+  describe('delete', () => {
+    it('should delete DB entry', async () => {
+      const output = {
+        taskId,
+        stdout: ['data'],
+        stderr: [],
+        totalSize: 4,
+      };
+      await repo.save(taskId, output);
+
+      const deleteResult = await repo.delete(taskId);
+      expect(deleteResult.ok).toBe(true);
+
+      const getResult = await repo.get(taskId);
+      expect(getResult.ok).toBe(true);
+      if (!getResult.ok) return;
+      expect(getResult.value).toBeNull();
+    });
+
+    it('should delete file-backed output and clean up file', async () => {
+      const largeData = 'x'.repeat(2048);
+      const output = {
+        taskId,
+        stdout: [largeData],
+        stderr: [],
+        totalSize: 2048,
+      };
+      await repo.save(taskId, output);
+
+      const deleteResult = await repo.delete(taskId);
+      expect(deleteResult.ok).toBe(true);
+
+      const getResult = await repo.get(taskId);
+      expect(getResult.ok).toBe(true);
+      if (!getResult.ok) return;
+      expect(getResult.value).toBeNull();
+    });
+
+    it('should succeed when deleting non-existent task', async () => {
+      const result = await repo.delete(TaskId('nonexistent'));
+      expect(result.ok).toBe(true);
+    });
+  });
+});

tests/unit/services/process-connector.test.ts

@@ -0,0 +1,145 @@
+/**
+ * Unit tests for ProcessConnector
+ *
+ * ARCHITECTURE: Tests stream wiring, exit handling, and double-exit guard
+ * Pattern: Mock ChildProcess (EventEmitter) + mock OutputCapture + TestLogger
+ */
+
+import { EventEmitter } from 'events';
+import { describe, expect, it, vi } from 'vitest';
+import type { TaskId } from '../../../src/core/domain.js';
+import type { Logger, OutputCapture } from '../../../src/core/interfaces.js';
+import { ok } from '../../../src/core/result.js';
+import { ProcessConnector } from '../../../src/services/process-connector.js';
+
+function createMockProcess(): EventEmitter & {
+  stdout: EventEmitter | null;
+  stderr: EventEmitter | null;
+} {
+  const proc = new EventEmitter() as EventEmitter & {
+    stdout: EventEmitter | null;
+    stderr: EventEmitter | null;
+  };
+  proc.stdout = new EventEmitter();
+  proc.stderr = new EventEmitter();
+  return proc;
+}
+
+function createTestLogger(): Logger {
+  return {
+    debug: vi.fn(),
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn(),
+    child: () => createTestLogger(),
+  };
+}
+
+function createMockOutputCapture(): OutputCapture {
+  return {
+    capture: vi.fn().mockReturnValue(ok(undefined)),
+    getOutput: vi.fn().mockReturnValue(ok({ taskId: 'test', stdout: [], stderr: [], totalSize: 0 })),
+    clear: vi.fn().mockReturnValue(ok(undefined)),
+  };
+}
+
+describe('ProcessConnector', () => {
+  const taskId = 'task-1' as TaskId;
+
+  it('should capture stdout data', () => {
+    const capture = createMockOutputCapture();
+    const logger = createTestLogger();
+    const connector = new ProcessConnector(capture, logger);
+    const proc = createMockProcess();
+    const onExit = vi.fn();
+
+    connector.connect(proc as never, taskId, onExit);
+    proc.stdout!.emit('data', Buffer.from('hello'));
+
+    expect(capture.capture).toHaveBeenCalledWith(taskId, 'stdout', 'hello');
+  });
+
+  it('should capture stderr data', () => {
+    const capture = createMockOutputCapture();
+    const logger = createTestLogger();
+    const connector = new ProcessConnector(capture, logger);
+    const proc = createMockProcess();
+    const onExit = vi.fn();
+
+    connector.connect(proc as never, taskId, onExit);
+    proc.stderr!.emit('data', Buffer.from('error output'));
+
+    expect(capture.capture).toHaveBeenCalledWith(taskId, 'stderr', 'error output');
+  });
+
+  it('should call onExit with exit code on process exit', () => {
+    const capture = createMockOutputCapture();
+    const logger = createTestLogger();
+    const connector = new ProcessConnector(capture, logger);
+    const proc = createMockProcess();
+    const onExit = vi.fn();
+
+    connector.connect(proc as never, taskId, onExit);
+    proc.emit('exit', 42);
+
+    expect(onExit).toHaveBeenCalledWith(42);
+  });
+
+  it('should preserve exit code 0 (nullish coalescing)', () => {
+    const capture = createMockOutputCapture();
+    const logger = createTestLogger();
+    const connector = new ProcessConnector(capture, logger);
+    const proc = createMockProcess();
+    const onExit = vi.fn();
+
+    connector.connect(proc as never, taskId, onExit);
+    proc.emit('exit', 0);
+
+    expect(onExit).toHaveBeenCalledWith(0);
+  });
+
+  it('should capture error and call onExit(1) on process error', () => {
+    const capture = createMockOutputCapture();
+    const logger = createTestLogger();
+    const connector = new ProcessConnector(capture, logger);
+    const proc = createMockProcess();
+    const onExit = vi.fn();
+
+    connector.connect(proc as never, taskId, onExit);
+    proc.emit('error', new Error('spawn failed'));
+
+    expect(capture.capture).toHaveBeenCalledWith(taskId, 'stderr', 'Process error: spawn failed\n');
+    expect(onExit).toHaveBeenCalledWith(1);
+  });
+
+  it('should prevent multiple onExit calls (double-exit guard)', () => {
+    const capture = createMockOutputCapture();
+    const logger = createTestLogger();
+    const connector = new ProcessConnector(capture, logger);
+    const proc = createMockProcess();
+    const onExit = vi.fn();
+
+    connector.connect(proc as never, taskId, onExit);
+    proc.emit('exit', 0);
+    proc.emit('exit', 1); // second exit should be ignored
+
+    expect(onExit).toHaveBeenCalledTimes(1);
+    expect(onExit).toHaveBeenCalledWith(0);
+  });
+
+  it('should handle process without stdout/stderr streams', () => {
+    const capture = createMockOutputCapture();
+    const logger = createTestLogger();
+    const connector = new ProcessConnector(capture, logger);
+    const proc = createMockProcess();
+    proc.stdout = null;
+    proc.stderr = null;
+    const onExit = vi.fn();
+
+    // Should not throw
+    connector.connect(proc as never, taskId, onExit);
+    proc.emit('exit', 0);
+
+    expect(onExit).toHaveBeenCalledWith(0);
+  });
+});