scriptoriumadmin/veritas-kanban
1
0
Fork 0
mirror of https://github.com/BradGroux/veritas-kanban.git synced 2026-08-28 02:44:59 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

refactor: move error analyses behind storage (#1200)

Browse source
This commit is contained in:
Brad Groux 2026-08-23 18:07:20 -05:00 • committed by GitHub
parent 06e1343958
commit f5e361ab8f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 342 additions and 95 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
docs/architecture/service-filesystem-boundary.json
View file

@ -1,6 +1,6 @@
{
"schemaVersion": 1,
"maximumEntries": 44,
"maximumEntries": 43,
"entries": [
{
"path": "server/src/services/agent-health-service.ts",
@ -62,12 +62,6 @@
"owner": "#1188",
"rationale": "Managed-content storage migration is tracked in issue #1188."
},
{
"path": "server/src/services/error-learning-service.ts",
"category": "authoritative-persistence",
"owner": "#1186",
"rationale": "Coordination-state storage migration is tracked in issue #1186."
},
{
"path": "server/src/services/external-tracker-service.ts",
"category": "authoritative-persistence",

1
docs/testing/critical-path-coverage.json
View file

@ -30,6 +30,7 @@
"src/__tests__/credential-broker-service.test.ts",
"src/__tests__/delegation-repository.test.ts",
"src/__tests__/enforcement.test.ts",
"src/__tests__/error-analysis-repository.test.ts",
"src/__tests__/filesystem-sandbox-service.test.ts",
"src/__tests__/harness-support-profile-schemas.test.ts",
"src/__tests__/hermes-provider.test.ts",

140
server/src/__tests__/error-analysis-repository.test.ts Normal file
View file

@ -0,0 +1,140 @@
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 { ErrorLearningService, type ErrorAnalysis } from '../services/error-learning-service.js';
import {
FileErrorAnalysisRepository,
InMemoryErrorAnalysisRepository,
} from '../storage/error-analysis-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 analysis(id: string): ErrorAnalysis {
return {
id,
context: { errorMessage: `Failure ${id}`, occurredAt: '2026-08-23T20:00:00.000Z' },
rootCause: '',
summary: `Failure ${id}`,
severity: 'medium',
optionsConsidered: [],
chosenFix: '',
preventionSteps: [],
tags: [],
relatedTasks: [],
isRepeat: false,
previousOccurrences: [],
analyzedAt: '2026-08-23T20:00:00.000Z',
};
}
describe('FileErrorAnalysisRepository', () => {
let root: string;
let runtimeDir: string;
let repository: FileErrorAnalysisRepository;
beforeEach(async () => {
root = await mkdtemp(path.join(process.cwd(), '.veritas-error-analysis-'));
runtimeDir = path.join(root, 'runtime');
repository = new FileErrorAnalysisRepository(runtimeDir, []);
});
afterEach(async () => {
await rm(root, { recursive: true, force: true });
});
it('reads defaults and serializes concurrent updates', async () => {
await expect(repository.read()).resolves.toEqual([]);
await Promise.all([
repository.update((analyses) => [...analyses, analysis('one')]),
repository.update((analyses) => [...analyses, analysis('two')]),
]);
expect((await repository.read()).map(({ id }) => id)).toEqual(
expect.arrayContaining(['one', 'two'])
);
});
it('migrates legacy data and tolerates malformed or non-array JSON', async () => {
const legacyDir = path.join(root, 'legacy');
await mkdir(legacyDir);
await writeFile(
path.join(legacyDir, 'error-analyses.json'),
JSON.stringify([analysis('legacy')]),
'utf8'
);
const migratingRepository = new FileErrorAnalysisRepository(runtimeDir, [legacyDir]);
await expect(migratingRepository.read()).resolves.toEqual([analysis('legacy')]);
await writeFile(path.join(runtimeDir, 'error-analyses.json'), '{broken', 'utf8');
await expect(repository.read()).resolves.toEqual([]);
await writeFile(path.join(runtimeDir, 'error-analyses.json'), '{}', 'utf8');
await expect(repository.read()).resolves.toEqual([]);
});
it('rejects symbolic links, changed files, and non-file paths', async () => {
await mkdir(runtimeDir, { recursive: true });
const target = path.join(root, 'outside.json');
await writeFile(target, '[]', 'utf8');
await symlink(target, path.join(runtimeDir, 'error-analyses.json'));
await expect(repository.read()).rejects.toThrow(/symbolic link/i);
await rm(path.join(runtimeDir, 'error-analyses.json'));
await writeFile(path.join(runtimeDir, 'error-analyses.json'), '[]', '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(path.join(runtimeDir, 'error-analyses.json'));
await mkdir(path.join(runtimeDir, 'error-analyses.json'));
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-runtime');
const linkedDirectory = path.join(root, 'linked-runtime');
await mkdir(realDirectory);
await symlink(realDirectory, linkedDirectory, 'dir');
const linkedRepository = new FileErrorAnalysisRepository(linkedDirectory, []);
await expect(linkedRepository.update(() => [analysis('unsafe')])).rejects.toThrow(
/regular directory/i
);
await expect(
repository.update(() => [{ ...analysis('large'), summary: 'x'.repeat(16 * 1024 * 1024) }])
).rejects.toThrow(/16 MiB/i);
});
});
describe('ErrorLearningService storage integration', () => {
it('creates, detects repeats, updates, lists, searches, and summarizes analyses', async () => {
const repository = new InMemoryErrorAnalysisRepository();
await repository.update(() => []);
const service = new ErrorLearningService(repository);
const first = await service.submitError({ errorMessage: 'Build failed with timeout' });
const second = await service.submitError({ errorMessage: 'Build failed with timeout again' });
expect(second.isRepeat).toBe(true);
const updated = await service.updateAnalysis(first.id, {
rootCause: 'Dependency unavailable',
preventionSteps: ['Retry with backoff'],
});
expect(updated?.rootCause).toBe('Dependency unavailable');
await expect(service.getAnalysis(first.id)).resolves.toMatchObject({
rootCause: 'Dependency unavailable',
});
await expect(service.listAnalyses({ severity: 'medium' })).resolves.toHaveLength(2);
await expect(service.searchSimilar('dependency unavailable')).resolves.toHaveLength(1);
await expect(service.getStats()).resolves.toMatchObject({
totalAnalyses: 2,
bySeverity: { medium: 2 },
repeatRate: 0.5,
});
});
});

148
server/src/services/error-learning-service.ts
View file

@ -14,13 +14,10 @@
import { getTaskService } from './task-service.js';
import { createLogger } from '../lib/logger.js';
import * as fs from 'node:fs/promises';
import * as path from 'node:path';
import { getLegacyRuntimeDirs, getRuntimeDir } from '../utils/paths.js';
import { migrateLegacyFiles } from '../utils/migrate-legacy-files.js';
const DATA_DIR = getRuntimeDir();
const LEGACY_DATA_DIRS = getLegacyRuntimeDirs();
let migrationChecked = false;
import {
FileErrorAnalysisRepository,
type ErrorAnalysisRepository,
} from '../storage/error-analysis-repository.js';
const log = createLogger('error-learning');
@ -102,38 +99,10 @@ export interface ErrorLearningStats {
// ─── Service ─────────────────────────────────────────────────────
class ErrorLearningService {
private analyses: ErrorAnalysis[] = [];
private loaded = false;
private get storagePath(): string {
return path.join(DATA_DIR, 'error-analyses.json');
}
private async ensureLoaded(): Promise<void> {
if (!migrationChecked) {
migrationChecked = true;
await migrateLegacyFiles(
LEGACY_DATA_DIRS,
DATA_DIR,
['error-analyses.json'],
'error analysis'
);
}
if (this.loaded) return;
try {
const data = await fs.readFile(this.storagePath, 'utf-8');
this.analyses = JSON.parse(data);
} catch {
this.analyses = [];
}
this.loaded = true;
}
private async save(): Promise<void> {
await fs.writeFile(this.storagePath, JSON.stringify(this.analyses, null, 2));
}
export class ErrorLearningService {
constructor(
private readonly repository: ErrorAnalysisRepository = new FileErrorAnalysisRepository()
) {}
/**
* Submit an error for analysis. Returns a structured analysis.
@ -143,33 +112,29 @@ class ErrorLearningService {
* or that can be completed via the API.
*/
async submitError(context: ErrorContext): Promise<ErrorAnalysis> {
await this.ensureLoaded();
// Check for previous similar errors
const previousOccurrences = this.findSimilarErrors(context);
const analysis: ErrorAnalysis = {
id: `err_${Date.now()}_${Math.random().toString(36).slice(2, 6)}`,
context: {
...context,
occurredAt: context.occurredAt || new Date().toISOString(),
},
rootCause: '', // To be filled by analyzing agent
summary: `Error in ${context.taskId || 'unknown task'}: ${context.errorMessage.slice(0, 200)}`,
severity: this.estimateSeverity(context),
optionsConsidered: [],
chosenFix: '',
preventionSteps: [],
tags: this.autoTag(context),
relatedTasks: context.taskId ? [context.taskId] : [],
isRepeat: previousOccurrences.length > 0,
previousOccurrences: previousOccurrences.map((a) => a.id),
analyzedAt: new Date().toISOString(),
analyzedBy: context.agent,
};
this.analyses.push(analysis);
await this.save();
const analysis = await this.repository.mutate((analyses) => {
const previousOccurrences = this.findSimilarErrors(analyses, context);
const created: ErrorAnalysis = {
id: `err_${Date.now()}_${Math.random().toString(36).slice(2, 6)}`,
context: {
...context,
occurredAt: context.occurredAt || new Date().toISOString(),
},
rootCause: '',
summary: `Error in ${context.taskId || 'unknown task'}: ${context.errorMessage.slice(0, 200)}`,
severity: this.estimateSeverity(context),
optionsConsidered: [],
chosenFix: '',
preventionSteps: [],
tags: this.autoTag(context),
relatedTasks: context.taskId ? [context.taskId] : [],
isRepeat: previousOccurrences.length > 0,
previousOccurrences: previousOccurrences.map((candidate) => candidate.id),
analyzedAt: new Date().toISOString(),
analyzedBy: context.agent,
};
return { analyses: [...analyses, created], result: created };
});
// If linked to a task, update the task's lessonsLearned
if (context.taskId) {
@ -204,14 +169,22 @@ class ErrorLearningService {
>
>
): Promise<ErrorAnalysis | null> {
await this.ensureLoaded();
const analysis = this.analyses.find((a) => a.id === id);
const analysis = await this.repository.mutate((analyses) => {
const index = analyses.findIndex((candidate) => candidate.id === id);
if (index === -1) {
return { analyses, result: null as ErrorAnalysis | null };
}
const updated: ErrorAnalysis = {
...analyses[index],
...update,
analyzedAt: new Date().toISOString(),
};
const next = [...analyses];
next[index] = updated;
return { analyses: next, result: updated as ErrorAnalysis | null };
});
if (!analysis) return null;
Object.assign(analysis, update, { analyzedAt: new Date().toISOString() });
await this.save();
// Update linked task
if (analysis.relatedTasks.length > 0) {
await this.linkToTask(analysis.relatedTasks[0], analysis);
@ -225,8 +198,8 @@ class ErrorLearningService {
* Get a specific analysis.
*/
async getAnalysis(id: string): Promise<ErrorAnalysis | null> {
await this.ensureLoaded();
return this.analyses.find((a) => a.id === id) || null;
const analyses = await this.repository.read();
return analyses.find((analysis) => analysis.id === id) || null;
}
/**
@ -239,12 +212,11 @@ class ErrorLearningService {
agent?: string;
limit?: number;
}): Promise<ErrorAnalysis[]> {
await this.ensureLoaded();
let results = [...this.analyses];
let results = [...(await this.repository.read())];
if (filters?.taskId) {
results = results.filter((a) => a.relatedTasks.includes(filters.taskId!));
const taskId = filters.taskId;
results = results.filter((analysis) => analysis.relatedTasks.includes(taskId));
}
if (filters?.errorType) {
results = results.filter((a) => a.context.errorType === filters.errorType);
@ -272,13 +244,13 @@ class ErrorLearningService {
* Get aggregate statistics about error patterns.
*/
async getStats(): Promise<ErrorLearningStats> {
await this.ensureLoaded();
const analyses = await this.repository.read();
const byType: Record<string, number> = {};
const bySeverity: Record<string, number> = {};
const preventionCounts: Record<string, number> = {};
for (const analysis of this.analyses) {
for (const analysis of analyses) {
// By type
const type = analysis.context.errorType || 'unknown';
byType[type] = (byType[type] || 0) + 1;
@ -292,15 +264,15 @@ class ErrorLearningService {
}
}
const repeats = this.analyses.filter((a) => a.isRepeat).length;
const repeatRate = this.analyses.length > 0 ? repeats / this.analyses.length : 0;
const repeats = analyses.filter((analysis) => analysis.isRepeat).length;
const repeatRate = analyses.length > 0 ? repeats / analyses.length : 0;
const topPreventionSteps = Object.entries(preventionCounts)
.map(([step, count]) => ({ step, count }))
.sort((a, b) => b.count - a.count)
.slice(0, 10);
const recentAnalyses = this.analyses
const recentAnalyses = analyses
.slice(-5)
.reverse()
.map((a) => ({
@ -311,7 +283,7 @@ class ErrorLearningService {
}));
return {
totalAnalyses: this.analyses.length,
totalAnalyses: analyses.length,
byType,
bySeverity,
repeatRate: Math.round(repeatRate * 100) / 100,
@ -325,7 +297,7 @@ class ErrorLearningService {
* Useful for agents to check "have we seen this before?"
*/
async searchSimilar(errorMessage: string, limit = 5): Promise<ErrorAnalysis[]> {
await this.ensureLoaded();
const analyses = await this.repository.read();
// Simple keyword matching — could be replaced with embeddings
const keywords = errorMessage
@ -333,7 +305,7 @@ class ErrorLearningService {
.split(/\s+/)
.filter((w) => w.length > 3);
const scored = this.analyses.map((analysis) => {
const scored = analyses.map((analysis) => {
const text =
`${analysis.context.errorMessage} ${analysis.rootCause} ${analysis.summary}`.toLowerCase();
const matches = keywords.filter((kw) => text.includes(kw)).length;
@ -349,9 +321,9 @@ class ErrorLearningService {
// ─── Private Helpers ─────────────────────────────────────────
private findSimilarErrors(context: ErrorContext): ErrorAnalysis[] {
private findSimilarErrors(analyses: ErrorAnalysis[], context: ErrorContext): ErrorAnalysis[] {
const msg = context.errorMessage.toLowerCase();
return this.analyses.filter((a) => {
return analyses.filter((a) => {
const existingMsg = a.context.errorMessage.toLowerCase();
// Check for significant overlap
const words = msg.split(/\s+/).filter((w) => w.length > 3);

135
server/src/storage/error-analysis-repository.ts Normal file
View file

@ -0,0 +1,135 @@
import { constants } from 'node:fs';
import { lstat, mkdir, open } from 'node:fs/promises';
import path from 'node:path';
import type { ErrorAnalysis } from '../services/error-learning-service.js';
import { withFileLock } from '../services/file-lock.js';
import { migrateLegacyFiles } from '../utils/migrate-legacy-files.js';
import { getLegacyRuntimeDirs, getRuntimeDir } from '../utils/paths.js';
import { ensureWithinBase } from '../utils/sanitize.js';
import { atomicWriteFile } from './fs-helpers.js';
const MAX_ERROR_ANALYSES_BYTES = 16 * 1024 * 1024;
export interface ErrorAnalysisRepository {
read(): Promise<ErrorAnalysis[]>;
update(updater: (analyses: ErrorAnalysis[]) => ErrorAnalysis[]): Promise<ErrorAnalysis[]>;
mutate<T>(
updater: (analyses: ErrorAnalysis[]) => { analyses: ErrorAnalysis[]; result: T }
): Promise<T>;
}
export class FileErrorAnalysisRepository implements ErrorAnalysisRepository {
private readonly runtimeDir: string;
private readonly analysisFile: string;
private migrationChecked = false;
constructor(
runtimeDir = getRuntimeDir(),
private readonly legacyRuntimeDirs: readonly string[] = getLegacyRuntimeDirs()
) {
this.runtimeDir = path.resolve(runtimeDir);
this.analysisFile = ensureWithinBase(
this.runtimeDir,
path.join(this.runtimeDir, 'error-analyses.json')
);
}
async read(): Promise<ErrorAnalysis[]> {
await this.ensureMigrated();
return this.readFile();
}
async update(updater: (analyses: ErrorAnalysis[]) => ErrorAnalysis[]): Promise<ErrorAnalysis[]> {
return this.mutate((analyses) => {
const updated = updater(analyses);
return { analyses: updated, result: updated };
});
}
async mutate<T>(
updater: (analyses: ErrorAnalysis[]) => { analyses: ErrorAnalysis[]; result: T }
): Promise<T> {
await this.ensureMigrated();
await this.prepareDirectory();
return withFileLock(this.analysisFile, async () => {
const { analyses, result } = updater(await this.readFile());
const content = JSON.stringify(analyses, null, 2);
if (Buffer.byteLength(content, 'utf8') > MAX_ERROR_ANALYSES_BYTES) {
throw new Error('Error analyses exceed the 16 MiB storage limit');
}
await atomicWriteFile(this.analysisFile, content, 'utf8');
return result;
});
}
private async ensureMigrated(): Promise<void> {
if (this.migrationChecked) return;
this.migrationChecked = true;
await migrateLegacyFiles(
this.legacyRuntimeDirs,
this.runtimeDir,
['error-analyses.json'],
'error analysis'
);
}
private async prepareDirectory(): Promise<void> {
await mkdir(this.runtimeDir, { recursive: true, mode: 0o700 });
const stats = await lstat(this.runtimeDir);
if (!stats.isDirectory() || stats.isSymbolicLink()) {
throw new Error('Error analysis storage path must use a regular directory');
}
}
private async readFile(): Promise<ErrorAnalysis[]> {
let handle: Awaited<ReturnType<typeof open>> | undefined;
try {
handle = await open(this.analysisFile, constants.O_RDONLY | (constants.O_NOFOLLOW ?? 0));
const [pathStats, stats] = await Promise.all([lstat(this.analysisFile), handle.stat()]);
if (
pathStats.isSymbolicLink() ||
pathStats.dev !== stats.dev ||
pathStats.ino !== stats.ino
) {
throw new Error('Error analyses must not use a symbolic link or changed file');
}
if (!stats.isFile() || stats.size > MAX_ERROR_ANALYSES_BYTES) {
throw new Error('Error analyses must use a bounded regular file');
}
const parsed: unknown = JSON.parse(await handle.readFile({ encoding: 'utf8' }));
return Array.isArray(parsed) ? (parsed as ErrorAnalysis[]) : [];
} catch (error) {
const errorCode = (error as NodeJS.ErrnoException).code;
if (errorCode === 'ENOENT' || error instanceof SyntaxError) return [];
if (errorCode === 'ELOOP') {
throw new Error('Error analyses must not use a symbolic link', { cause: error });
}
throw error;
} finally {
await handle?.close();
}
}
}
export class InMemoryErrorAnalysisRepository implements ErrorAnalysisRepository {
private analyses: ErrorAnalysis[] = [];
async read(): Promise<ErrorAnalysis[]> {
return this.analyses;
}
async update(updater: (analyses: ErrorAnalysis[]) => ErrorAnalysis[]): Promise<ErrorAnalysis[]> {
return this.mutate((analyses) => {
const updated = updater(analyses);
return { analyses: updated, result: updated };
});
}
async mutate<T>(
updater: (analyses: ErrorAnalysis[]) => { analyses: ErrorAnalysis[]; result: T }
): Promise<T> {
const mutation = updater(this.analyses);
this.analyses = mutation.analyses;
return mutation.result;
}
}

5
server/src/storage/index.ts
View file

@ -50,6 +50,11 @@ export {
type CeremonyStateRepository,
} from './ceremony-state-repository.js';
export { FileDelegationRepository, type DelegationRepository } from './delegation-repository.js';
export {
FileErrorAnalysisRepository,
InMemoryErrorAnalysisRepository,
type ErrorAnalysisRepository,
} from './error-analysis-repository.js';
export {
LocalConflictWorkspaceRepository,
type ConflictWorkspaceRepository,