refactor: move reflection state behind storage (#1205)

docs/architecture/service-filesystem-boundary.json

@@ -1,6 +1,6 @@
 {
   "schemaVersion": 1,
-  "maximumEntries": 39,
+  "maximumEntries": 38,
   "entries": [
     {
       "path": "server/src/services/agent-health-service.ts",
@@ -140,12 +140,6 @@
       "owner": "#1187",
       "rationale": "Operational evidence storage migration is tracked in issue #1187."
     },
-    {
-      "path": "server/src/services/reflection-service.ts",
-      "category": "authoritative-persistence",
-      "owner": "#1186",
-      "rationale": "Coordination-state storage migration is tracked in issue #1186."
-    },
     {
       "path": "server/src/services/run-session-share-service.ts",
       "category": "authoritative-persistence",

docs/testing/critical-path-coverage.json

@@ -50,6 +50,7 @@
           "src/__tests__/provider-runtime-manifest-service.test.ts",
           "src/__tests__/provider-task-envelope-renderer.test.ts",
           "src/__tests__/reflection-extraction-job-service.test.ts",
+          "src/__tests__/reflection-state-repository.test.ts",
           "src/__tests__/runtime-paths.test.ts",
           "src/__tests__/scheduler-state-repository.test.ts",
           "src/__tests__/scheduled-deliverables-repository.test.ts",

server/src/__tests__/reflection-state-repository.test.ts

@@ -0,0 +1,112 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { lstat, mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises';
+import path from 'node:path';
+import type { ReflectionCandidate } from '@veritas-kanban/shared';
+import { FileReflectionStateRepository } from '../storage/reflection-state-repository.js';
+
+vi.mock('node:fs/promises', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:fs/promises')>();
+  return { ...actual, lstat: vi.fn(actual.lstat) };
+});
+
+function candidate(id: string, summary = id): ReflectionCandidate {
+  return {
+    id,
+    status: 'pending',
+    category: 'team',
+    promotionTarget: 'memory',
+    confidence: 0.5,
+    source: { kind: 'user-correction' },
+    summary,
+    previousApproach: 'old',
+    correction: 'new',
+    nextAttempt: 'retry',
+    evidence: [],
+    tags: [],
+    duplicateKey: id,
+    duplicateCount: 1,
+    appliedTargets: [],
+    redaction: { redacted: false, notes: [] },
+    createdAt: '2026-08-23T00:00:00.000Z',
+    updatedAt: '2026-08-23T00:00:00.000Z',
+  };
+}
+
+describe('FileReflectionStateRepository', () => {
+  let root: string;
+  let storageDir: string;
+  let repository: FileReflectionStateRepository;
+
+  beforeEach(async () => {
+    root = await mkdtemp(path.join(process.cwd(), '.veritas-reflection-state-'));
+    storageDir = path.join(root, 'reflections');
+    repository = new FileReflectionStateRepository(storageDir);
+  });
+
+  afterEach(async () => {
+    await rm(root, { recursive: true, force: true });
+  });
+
+  it('reads missing state and atomically replaces normalized state', async () => {
+    await expect(repository.read()).resolves.toBeNull();
+    const state = {
+      version: 1 as const,
+      candidates: [candidate('one')],
+      updatedAt: '2026-08-23T00:00:00.000Z',
+    };
+    await repository.write(state);
+    await expect(repository.read()).resolves.toEqual(state);
+    await repository.write({ ...state, candidates: [candidate('two')] });
+    await expect(repository.read()).resolves.toEqual({
+      ...state,
+      candidates: [candidate('two')],
+    });
+  });
+
+  it('rejects symbolic links, changed files, and non-file paths', async () => {
+    await mkdir(storageDir, { recursive: true });
+    const stateFile = path.join(storageDir, 'candidates.json');
+    const target = path.join(root, 'outside.json');
+    await writeFile(target, JSON.stringify({ version: 1, candidates: [] }), 'utf8');
+    await symlink(target, stateFile);
+    await expect(repository.read()).rejects.toThrow(/symbolic link/i);
+
+    await rm(stateFile);
+    await writeFile(stateFile, JSON.stringify({ version: 1, candidates: [] }), 'utf8');
+    const actual = await vi.importActual<typeof import('node:fs/promises')>('node:fs/promises');
+    vi.mocked(lstat).mockImplementationOnce(async (filePath) => {
+      const stats = await actual.lstat(filePath);
+      return Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, {
+        ino: stats.ino + 1,
+      });
+    });
+    await expect(repository.read()).rejects.toThrow(/changed file/i);
+
+    await rm(stateFile);
+    await mkdir(stateFile);
+    await expect(repository.read()).rejects.toThrow(/bounded regular file/i);
+  });
+
+  it('rejects symbolic-link directories and oversized state', async () => {
+    const realDirectory = path.join(root, 'real-reflections');
+    const linkedDirectory = path.join(root, 'linked-reflections');
+    await mkdir(realDirectory);
+    await symlink(realDirectory, linkedDirectory, 'dir');
+    const linkedRepository = new FileReflectionStateRepository(linkedDirectory);
+    await expect(
+      linkedRepository.write({
+        version: 1,
+        candidates: [],
+        updatedAt: '2026-08-23T00:00:00.000Z',
+      })
+    ).rejects.toThrow(/regular directory/i);
+
+    await expect(
+      repository.write({
+        version: 1,
+        candidates: [candidate('large', 'x'.repeat(16 * 1024 * 1024))],
+        updatedAt: '2026-08-23T00:00:00.000Z',
+      })
+    ).rejects.toThrow(/16 MiB/i);
+  });
+});

server/src/services/reflection-service.ts

@@ -1,4 +1,3 @@
-import fs from 'fs/promises';
 import path from 'path';
 import { nanoid } from 'nanoid';
 import type {
@@ -22,12 +21,16 @@ import type {
   TaskEnvelopeMemoryReference,
 } from '@veritas-kanban/shared';
 import { auditLog, type AuditEvent } from './audit-service.js';
-import { withFileLock } from './file-lock.js';
 import { getTaskService } from './task-service.js';
 import { ConflictError, NotFoundError } from '../middleware/error-handler.js';
-import { ensureWithinBase, stripHtml, validatePathSegment } from '../utils/sanitize.js';
+import { stripHtml, validatePathSegment } from '../utils/sanitize.js';
 import { getRuntimeDir } from '../utils/paths.js';
 import { createLogger } from '../lib/logger.js';
+import {
+  FileReflectionStateRepository,
+  type ReflectionState,
+  type ReflectionStateRepository,
+} from '../storage/reflection-state-repository.js';
 import {
   getReflectionPromotionAdapterRegistry,
   type ReflectionPromotionAdapterRegistry,
@@ -43,12 +46,6 @@ const MAX_SOURCE_EVENT_IDS = 20;
 const MAX_RETRIEVALS_PER_CANDIDATE = 100;
 const DEFAULT_RETRIEVAL_LIMIT = 8;
 
-interface ReflectionState {
-  version: 1;
-  candidates: ReflectionCandidate[];
-  updatedAt: string;
-}
-
 export interface ReflectionListFilters {
   status?: ReflectionCandidateStatus;
   category?: ReflectionCandidateCategory;
@@ -76,6 +73,7 @@ export interface ReflectionTaskService {
 
 export interface ReflectionServiceOptions {
   storageDir?: string;
+  stateRepository?: ReflectionStateRepository;
   persist?: boolean;
   audit?: (event: AuditEvent) => Promise<void>;
   taskService?: ReflectionTaskService;
@@ -143,7 +141,7 @@ function lessonEntry(
 }
 
 export class ReflectionService {
-  private readonly storageDir: string;
+  private readonly stateRepository: ReflectionStateRepository;
   private readonly persist: boolean;
   private readonly audit: (event: AuditEvent) => Promise<void>;
   private readonly taskService: ReflectionTaskService;
@@ -152,7 +150,8 @@ export class ReflectionService {
   private state: ReflectionState = this.emptyState();
 
   constructor(options: ReflectionServiceOptions = {}) {
-    this.storageDir = options.storageDir ?? path.join(getRuntimeDir(), 'reflections');
+    const storageDir = options.storageDir ?? path.join(getRuntimeDir(), 'reflections');
+    this.stateRepository = options.stateRepository ?? new FileReflectionStateRepository(storageDir);
     this.persist = options.persist ?? process.env.VITEST !== 'true';
     this.audit = options.audit ?? auditLog;
     this.taskService = options.taskService ?? getTaskService();
@@ -664,20 +663,11 @@ export class ReflectionService {
       return;
     }
 
-    await fs.mkdir(this.storageDir, { recursive: true });
-    try {
-      const raw = await fs.readFile(this.statePath, 'utf-8');
-      const parsed = JSON.parse(raw) as Partial<ReflectionState>;
-      this.state = {
-        version: 1,
-        candidates: Array.isArray(parsed.candidates)
-          ? (parsed.candidates as ReflectionCandidate[])
-          : [],
-        updatedAt: typeof parsed.updatedAt === 'string' ? parsed.updatedAt : nowIso(),
-      };
+    const state = await this.stateRepository.read();
+    if (state) {
+      this.state = state;
       this.refreshDuplicateCounts();
-    } catch (err) {
-      if ((err as NodeJS.ErrnoException).code !== 'ENOENT') throw err;
+    } else {
       this.state = this.emptyState();
     }
     this.loaded = true;
@@ -686,16 +676,7 @@ export class ReflectionService {
   private async saveState(): Promise<void> {
     this.state.updatedAt = nowIso();
     if (!this.persist) return;
-    await fs.mkdir(this.storageDir, { recursive: true });
-    await withFileLock(this.statePath, async () => {
-      await fs.writeFile(this.statePath, JSON.stringify(this.state, null, 2), 'utf-8');
-    });
-  }
-
-  private get statePath(): string {
-    const filePath = path.join(this.storageDir, 'candidates.json');
-    ensureWithinBase(this.storageDir, filePath);
-    return filePath;
+    await this.stateRepository.write(this.state);
   }
 
   private emptyState(): ReflectionState {

server/src/storage/index.ts

@@ -55,6 +55,11 @@ export {
   type SchedulerState,
   type SchedulerStateRepository,
 } from './scheduler-state-repository.js';
+export {
+  FileReflectionStateRepository,
+  type ReflectionState,
+  type ReflectionStateRepository,
+} from './reflection-state-repository.js';
 export { FileScheduledDeliverablesStore } from './scheduled-deliverables-repository.js';
 export { FileBroadcastRepository } from './broadcast-repository.js';
 export {

server/src/storage/reflection-state-repository.ts

@@ -0,0 +1,84 @@
+import { constants } from 'node:fs';
+import { lstat, mkdir, open } from 'node:fs/promises';
+import path from 'node:path';
+import type { ReflectionCandidate } from '@veritas-kanban/shared';
+import { withFileLock } from '../services/file-lock.js';
+import { ensureWithinBase } from '../utils/sanitize.js';
+import { atomicWriteFile } from './fs-helpers.js';
+
+const MAX_REFLECTION_STATE_BYTES = 16 * 1024 * 1024;
+
+export interface ReflectionState {
+  version: 1;
+  candidates: ReflectionCandidate[];
+  updatedAt: string;
+}
+
+export interface ReflectionStateRepository {
+  read(): Promise<ReflectionState | null>;
+  write(state: ReflectionState): Promise<void>;
+}
+
+function normalizeState(parsed: Partial<ReflectionState>): ReflectionState {
+  return {
+    version: 1,
+    candidates: Array.isArray(parsed.candidates)
+      ? (parsed.candidates as ReflectionCandidate[])
+      : [],
+    updatedAt: typeof parsed.updatedAt === 'string' ? parsed.updatedAt : new Date().toISOString(),
+  };
+}
+
+export class FileReflectionStateRepository implements ReflectionStateRepository {
+  private readonly storageDir: string;
+  private readonly stateFile: string;
+
+  constructor(storageDir: string) {
+    this.storageDir = path.resolve(storageDir);
+    this.stateFile = ensureWithinBase(
+      this.storageDir,
+      path.join(this.storageDir, 'candidates.json')
+    );
+  }
+
+  async read(): Promise<ReflectionState | null> {
+    let handle: Awaited<ReturnType<typeof open>> | undefined;
+    try {
+      handle = await open(this.stateFile, constants.O_RDONLY | (constants.O_NOFOLLOW ?? 0));
+      const [pathStats, stats] = await Promise.all([lstat(this.stateFile), handle.stat()]);
+      if (
+        pathStats.isSymbolicLink() ||
+        pathStats.dev !== stats.dev ||
+        pathStats.ino !== stats.ino
+      ) {
+        throw new Error('Reflection state must not use a symbolic link or changed file');
+      }
+      if (!stats.isFile() || stats.size > MAX_REFLECTION_STATE_BYTES) {
+        throw new Error('Reflection state must use a bounded regular file');
+      }
+      return normalizeState(JSON.parse(await handle.readFile({ encoding: 'utf8' })));
+    } catch (error) {
+      const errorCode = (error as NodeJS.ErrnoException).code;
+      if (errorCode === 'ENOENT') return null;
+      if (errorCode === 'ELOOP') {
+        throw new Error('Reflection state must not use a symbolic link', { cause: error });
+      }
+      throw error;
+    } finally {
+      await handle?.close();
+    }
+  }
+
+  async write(state: ReflectionState): Promise<void> {
+    await mkdir(this.storageDir, { recursive: true, mode: 0o700 });
+    const directoryStats = await lstat(this.storageDir);
+    if (!directoryStats.isDirectory() || directoryStats.isSymbolicLink()) {
+      throw new Error('Reflection state path must use a regular directory');
+    }
+    const content = JSON.stringify(state, null, 2);
+    if (Buffer.byteLength(content, 'utf8') > MAX_REFLECTION_STATE_BYTES) {
+      throw new Error('Reflection state exceeds the 16 MiB storage limit');
+    }
+    await withFileLock(this.stateFile, () => atomicWriteFile(this.stateFile, content, 'utf8'));
+  }
+}