fix(FN-2349): migrate agent-generation diagnostics to shared helper
- Replace raw console diagnostics in agent-generation with structured ai-session diagnostics events - Add a test-only cleanup hook and regression tests for cleanup telemetry and AI generation failure logging - Expand diagnostics guardrail coverage to include agent-generation and generalize guardrail naming to AI-session modules
This commit is contained in:
@@ -8,9 +8,16 @@ import {
|
|||||||
getRateLimitResetTime,
|
getRateLimitResetTime,
|
||||||
parseGenerationResponse,
|
parseGenerationResponse,
|
||||||
__resetAgentGenerationState,
|
__resetAgentGenerationState,
|
||||||
|
__runAgentGenerationCleanupForTests,
|
||||||
RateLimitError,
|
RateLimitError,
|
||||||
SessionNotFoundError,
|
SessionNotFoundError,
|
||||||
} from "./agent-generation.js";
|
} from "./agent-generation.js";
|
||||||
|
import {
|
||||||
|
setDiagnosticsSink,
|
||||||
|
resetDiagnosticsSink,
|
||||||
|
type DiagnosticsContext,
|
||||||
|
type DiagnosticsLevel,
|
||||||
|
} from "./ai-session-diagnostics.js";
|
||||||
|
|
||||||
// Counter for unique IPs per test
|
// Counter for unique IPs per test
|
||||||
let ipCounter = 0;
|
let ipCounter = 0;
|
||||||
@@ -18,6 +25,13 @@ function getUniqueIp(): string {
|
|||||||
return `127.0.0.${++ipCounter}`;
|
return `127.0.0.${++ipCounter}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
interface CapturedDiagnostic {
|
||||||
|
level: DiagnosticsLevel;
|
||||||
|
scope: string;
|
||||||
|
message: string;
|
||||||
|
context: DiagnosticsContext;
|
||||||
|
}
|
||||||
|
|
||||||
describe("agent-generation module", () => {
|
describe("agent-generation module", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vi.useFakeTimers();
|
vi.useFakeTimers();
|
||||||
@@ -25,7 +39,9 @@ describe("agent-generation module", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
|
resetDiagnosticsSink();
|
||||||
vi.useRealTimers();
|
vi.useRealTimers();
|
||||||
|
vi.restoreAllMocks();
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("startAgentGeneration", () => {
|
describe("startAgentGeneration", () => {
|
||||||
@@ -101,6 +117,45 @@ describe("agent-generation module", () => {
|
|||||||
generateAgentSpec("non-existent-session-id", "/tmp")
|
generateAgentSpec("non-existent-session-id", "/tmp")
|
||||||
).rejects.toThrow(SessionNotFoundError);
|
).rejects.toThrow(SessionNotFoundError);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("logs structured error diagnostics with sessionId and rethrows when AI generation fails", async () => {
|
||||||
|
vi.resetModules();
|
||||||
|
|
||||||
|
const generationFailure = new Error("engine failed to generate spec");
|
||||||
|
vi.doMock("@fusion/engine", () => ({
|
||||||
|
createFnAgent: vi.fn(async () => {
|
||||||
|
throw generationFailure;
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
|
||||||
|
const diagnostics: CapturedDiagnostic[] = [];
|
||||||
|
|
||||||
|
const diagnosticsModule = await import("./ai-session-diagnostics.js");
|
||||||
|
diagnosticsModule.setDiagnosticsSink((level, scope, message, context) => {
|
||||||
|
diagnostics.push({ level, scope, message, context });
|
||||||
|
});
|
||||||
|
|
||||||
|
const agentGenerationModule = await import("./agent-generation.js");
|
||||||
|
const session = await agentGenerationModule.startAgentGeneration(getUniqueIp(), "Role requiring AI spec");
|
||||||
|
|
||||||
|
await expect(agentGenerationModule.generateAgentSpec(session.id, "/tmp")).rejects.toThrow(generationFailure);
|
||||||
|
|
||||||
|
expect(diagnostics).toHaveLength(1);
|
||||||
|
expect(diagnostics[0]).toMatchObject({
|
||||||
|
level: "error",
|
||||||
|
scope: "agent-generation",
|
||||||
|
message: "AI generation failed for session",
|
||||||
|
context: expect.objectContaining({
|
||||||
|
sessionId: session.id,
|
||||||
|
operation: "generate-agent-spec",
|
||||||
|
error: expect.objectContaining({
|
||||||
|
message: "engine failed to generate spec",
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
|
||||||
|
diagnosticsModule.resetDiagnosticsSink();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("getAgentGenerationSession", () => {
|
describe("getAgentGenerationSession", () => {
|
||||||
@@ -334,6 +389,36 @@ describe("agent-generation module", () => {
|
|||||||
expect(retrieved).toBeDefined();
|
expect(retrieved).toBeDefined();
|
||||||
expect(retrieved!.roleDescription).toBe("Security auditor role");
|
expect(retrieved!.roleDescription).toBe("Security auditor role");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("emits one structured cleanup diagnostic when expired sessions and stale rate limits are removed", async () => {
|
||||||
|
const diagnostics: CapturedDiagnostic[] = [];
|
||||||
|
setDiagnosticsSink((level, scope, message, context) => {
|
||||||
|
diagnostics.push({ level, scope, message, context });
|
||||||
|
});
|
||||||
|
|
||||||
|
const staleIp = getUniqueIp();
|
||||||
|
const session = await startAgentGeneration(staleIp, "Temporary role");
|
||||||
|
checkRateLimit("192.168.0.99");
|
||||||
|
|
||||||
|
vi.setSystemTime(Date.now() + 61 * 60 * 1000);
|
||||||
|
__runAgentGenerationCleanupForTests();
|
||||||
|
|
||||||
|
expect(getAgentGenerationSession(session.id)).toBeUndefined();
|
||||||
|
expect(getRateLimitResetTime(staleIp)).toBeNull();
|
||||||
|
expect(getRateLimitResetTime("192.168.0.99")).toBeNull();
|
||||||
|
|
||||||
|
expect(diagnostics).toHaveLength(1);
|
||||||
|
expect(diagnostics[0]).toMatchObject({
|
||||||
|
level: "info",
|
||||||
|
scope: "agent-generation",
|
||||||
|
message: "Cleanup completed",
|
||||||
|
context: expect.objectContaining({
|
||||||
|
cleanedSessions: 1,
|
||||||
|
cleanedRateLimits: 2,
|
||||||
|
operation: "cleanup-expired",
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("prompt override support", () => {
|
describe("prompt override support", () => {
|
||||||
|
|||||||
@@ -13,6 +13,7 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import { randomUUID } from "node:crypto";
|
import { randomUUID } from "node:crypto";
|
||||||
|
import { createSessionDiagnostics } from "./ai-session-diagnostics.js";
|
||||||
|
|
||||||
// Dynamic import for @fusion/core to get prompt override resolution
|
// Dynamic import for @fusion/core to get prompt override resolution
|
||||||
|
|
||||||
@@ -183,6 +184,9 @@ const sessions = new Map<string, Session>();
|
|||||||
/** Rate limiting state indexed by IP */
|
/** Rate limiting state indexed by IP */
|
||||||
const rateLimits = new Map<string, RateLimitEntry>();
|
const rateLimits = new Map<string, RateLimitEntry>();
|
||||||
|
|
||||||
|
/** Shared diagnostics helper for structured agent-generation telemetry. */
|
||||||
|
const diagnostics = createSessionDiagnostics("agent-generation");
|
||||||
|
|
||||||
// ── Cleanup Interval ────────────────────────────────────────────────────────
|
// ── Cleanup Interval ────────────────────────────────────────────────────────
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -208,12 +212,22 @@ function cleanupExpiredSessions(): void {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (cleanedSessions > 0 || cleanedRateLimits > 0) {
|
if (cleanedSessions > 0 || cleanedRateLimits > 0) {
|
||||||
console.log(
|
diagnostics.info("Cleanup completed", {
|
||||||
`[agent-generation] Cleanup: removed ${cleanedSessions} sessions, ${cleanedRateLimits} rate limit entries`
|
cleanedSessions,
|
||||||
);
|
cleanedRateLimits,
|
||||||
|
operation: "cleanup-expired",
|
||||||
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Run expiry cleanup immediately. Test-only helper for deterministic assertions.
|
||||||
|
* @internal
|
||||||
|
*/
|
||||||
|
export function __runAgentGenerationCleanupForTests(): void {
|
||||||
|
cleanupExpiredSessions();
|
||||||
|
}
|
||||||
|
|
||||||
const cleanupInterval = setInterval(cleanupExpiredSessions, CLEANUP_INTERVAL_MS);
|
const cleanupInterval = setInterval(cleanupExpiredSessions, CLEANUP_INTERVAL_MS);
|
||||||
cleanupInterval.unref?.();
|
cleanupInterval.unref?.();
|
||||||
|
|
||||||
@@ -472,7 +486,10 @@ export async function generateAgentSpec(
|
|||||||
session.updatedAt = new Date();
|
session.updatedAt = new Date();
|
||||||
return spec;
|
return spec;
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
console.error(`[agent-generation] AI generation failed for session ${sessionId}:`, err);
|
diagnostics.errorFromException("AI generation failed for session", err, {
|
||||||
|
sessionId,
|
||||||
|
operation: "generate-agent-spec",
|
||||||
|
});
|
||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
/**
|
/**
|
||||||
* Planning-Flow Diagnostics Guardrail Test
|
* AI-Session Diagnostics Guardrail Test
|
||||||
*
|
*
|
||||||
* This test enforces that AI-session planning flow modules use the shared
|
* This test enforces that AI-session modules use the shared
|
||||||
* ai-session-diagnostics helper instead of raw console.* calls for diagnostics.
|
* ai-session-diagnostics helper instead of raw console.* calls for diagnostics.
|
||||||
*
|
*
|
||||||
* Guardrail: These modules must NOT contain direct console.log( / console.warn( / console.error(
|
* Guardrail: These modules must NOT contain direct console.log( / console.warn( / console.error(
|
||||||
@@ -18,17 +18,18 @@ import { readFileSync } from "node:fs";
|
|||||||
import { resolve } from "node:path";
|
import { resolve } from "node:path";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* List of planning-flow modules that must use the shared diagnostics helper.
|
* List of AI-session modules that must use the shared diagnostics helper.
|
||||||
* These modules handle AI-session flows and must not use raw console.* diagnostics.
|
* These modules handle AI-session flows and must not use raw console.* diagnostics.
|
||||||
*/
|
*/
|
||||||
const PLANNING_FLOW_MODULES = [
|
const AI_SESSION_FLOW_MODULES = [
|
||||||
"planning.ts",
|
"planning.ts",
|
||||||
"mission-interview.ts",
|
"mission-interview.ts",
|
||||||
"milestone-slice-interview.ts",
|
"milestone-slice-interview.ts",
|
||||||
"subtask-breakdown.ts",
|
"subtask-breakdown.ts",
|
||||||
|
"agent-generation.ts",
|
||||||
] as const;
|
] as const;
|
||||||
|
|
||||||
type PlanningFlowModule = (typeof PLANNING_FLOW_MODULES)[number];
|
type AiSessionFlowModule = (typeof AI_SESSION_FLOW_MODULES)[number];
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Patterns that indicate raw console diagnostics in AI-session failure paths.
|
* Patterns that indicate raw console diagnostics in AI-session failure paths.
|
||||||
@@ -44,10 +45,10 @@ const RAW_CONSOLE_PATTERNS: readonly RegExp[] = [
|
|||||||
type RawConsolePattern = (typeof RAW_CONSOLE_PATTERNS)[number];
|
type RawConsolePattern = (typeof RAW_CONSOLE_PATTERNS)[number];
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Read the source content of a planning-flow module.
|
* Read the source content of an AI-session flow module.
|
||||||
* Throws if the file cannot be read.
|
* Throws if the file cannot be read.
|
||||||
*/
|
*/
|
||||||
function readModuleSource(moduleName: PlanningFlowModule): string {
|
function readModuleSource(moduleName: AiSessionFlowModule): string {
|
||||||
const modulePath = resolve(import.meta.dirname, moduleName);
|
const modulePath = resolve(import.meta.dirname, moduleName);
|
||||||
return readFileSync(modulePath, "utf-8");
|
return readFileSync(modulePath, "utf-8");
|
||||||
}
|
}
|
||||||
@@ -72,18 +73,18 @@ function findRawConsoleCalls(
|
|||||||
return violations;
|
return violations;
|
||||||
}
|
}
|
||||||
|
|
||||||
describe("Planning-Flow Diagnostics Guardrail", () => {
|
describe("AI-Session Diagnostics Guardrail", () => {
|
||||||
/**
|
/**
|
||||||
* Test that each planning-flow module uses the shared diagnostics helper
|
* Test that each AI-session flow module uses the shared diagnostics helper
|
||||||
* instead of raw console.* calls.
|
* instead of raw console.* calls.
|
||||||
*
|
*
|
||||||
* This guardrail prevents:
|
* This guardrail prevents:
|
||||||
* - Incomplete migrations where raw console.* remains
|
* - Incomplete migrations where raw console.* remains
|
||||||
* - Accidental reintroduction of raw console diagnostics
|
* - Accidental reintroduction of raw console diagnostics
|
||||||
* - Inconsistent diagnostics across planning flow modules
|
* - Inconsistent diagnostics across AI-session modules
|
||||||
*/
|
*/
|
||||||
describe("AI-session failure paths must use shared diagnostics helper", () => {
|
describe("AI-session failure paths must use shared diagnostics helper", () => {
|
||||||
for (const moduleName of PLANNING_FLOW_MODULES) {
|
for (const moduleName of AI_SESSION_FLOW_MODULES) {
|
||||||
const moduleShortName = moduleName.replace(".ts", "");
|
const moduleShortName = moduleName.replace(".ts", "");
|
||||||
|
|
||||||
it(`${moduleShortName} does not contain raw console.* diagnostics`, () => {
|
it(`${moduleShortName} does not contain raw console.* diagnostics`, () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user