refactor(FN-2129): route agent-instructions warnings through structured logger
- Replace direct console.warn calls in agent-instructions with createLogger("agent-instructions") warnings
- Keep existing truncation and path-validation safeguards while standardizing diagnostic output format
- Add focused diagnostics tests that verify single warning emission for truncation and invalid instructionsPath scenarios
- Add coverage for missing instructions files and project memory read failures to ensure graceful fallback behavior
This commit is contained in:
@@ -552,3 +552,151 @@ describe("buildSystemPromptWithInstructions", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("diagnostics logging", () => {
|
||||
let testDir: string;
|
||||
|
||||
beforeEach(async () => {
|
||||
testDir = await mkdtemp(join(tmpdir(), "agent-instr-diagnostics-"));
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
vi.restoreAllMocks();
|
||||
await rm(testDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("logs exactly once when oversized instructionsText is truncated", async () => {
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const agent = makeAgent({ instructionsText: "x".repeat(50_010) });
|
||||
|
||||
const result = await resolveAgentInstructions(agent, testDir);
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("agent-instructions");
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("instructionsText exceeded max length");
|
||||
expect(result.length).toBe(50_000);
|
||||
});
|
||||
|
||||
it("logs exactly once when oversized instructionsPath file content is truncated", async () => {
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
await writeFile(join(testDir, "large.md"), "y".repeat(50_020));
|
||||
const agent = makeAgent({ instructionsPath: "large.md" });
|
||||
|
||||
const result = await resolveAgentInstructions(agent, testDir);
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("agent-instructions");
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("instructions file content exceeded max length");
|
||||
expect(result.length).toBe(50_000);
|
||||
});
|
||||
|
||||
it("logs exactly once when oversized soul is truncated", async () => {
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const agent = makeAgent({ soul: "s".repeat(10_010) });
|
||||
|
||||
const result = await resolveAgentInstructions(agent, testDir);
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("agent-instructions");
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("soul exceeded max length");
|
||||
expect(result.length).toBe(10_000 + "## Soul\n\n".length);
|
||||
});
|
||||
|
||||
it("logs exactly once when oversized memory is truncated", async () => {
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const memory = "m".repeat(50_010);
|
||||
const agent = makeAgent({ memory });
|
||||
|
||||
const result = await resolveAgentInstructions(agent, testDir);
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("agent-instructions");
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("memory exceeded max length");
|
||||
expect(result).toContain("## Agent Memory");
|
||||
expect(result).toContain(memory.slice(0, 50_000));
|
||||
});
|
||||
|
||||
it("logs exactly once when instructionsPath is too long", async () => {
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const agent = makeAgent({ instructionsPath: `${"a".repeat(501)}.md` });
|
||||
|
||||
const result = await resolveAgentInstructions(agent, testDir);
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("agent-instructions");
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("instructionsPath too long");
|
||||
expect(result).toBe("");
|
||||
});
|
||||
|
||||
it("logs exactly once when instructionsPath does not end in .md", async () => {
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
await writeFile(join(testDir, "instructions.txt"), "plain text");
|
||||
const agent = makeAgent({ instructionsPath: "instructions.txt" });
|
||||
|
||||
const result = await resolveAgentInstructions(agent, testDir);
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("agent-instructions");
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("must end in .md");
|
||||
expect(result).toBe("");
|
||||
});
|
||||
|
||||
it("logs exactly once when instructionsPath is absolute", async () => {
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const agent = makeAgent({ instructionsPath: "/etc/passwd.md" });
|
||||
|
||||
const result = await resolveAgentInstructions(agent, testDir);
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("agent-instructions");
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("must be project-relative");
|
||||
expect(result).toBe("");
|
||||
});
|
||||
|
||||
it("logs exactly once when instructionsPath attempts traversal", async () => {
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const agent = makeAgent({ instructionsPath: "../secrets.md" });
|
||||
|
||||
const result = await resolveAgentInstructions(agent, testDir);
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("agent-instructions");
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("traversal is not allowed");
|
||||
expect(result).toBe("");
|
||||
});
|
||||
|
||||
it("logs exactly once when instructionsPath file is missing", async () => {
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const agent = makeAgent({
|
||||
instructionsText: "Fallback text.",
|
||||
instructionsPath: "nonexistent.md",
|
||||
});
|
||||
|
||||
const result = await resolveAgentInstructions(agent, testDir);
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("agent-instructions");
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("file not found");
|
||||
expect(result).toBe("Fallback text.");
|
||||
});
|
||||
|
||||
it("logs exactly once when project memory read fails in buildAgentChatPrompt", async () => {
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
await mkdir(join(testDir, ".fusion", "memory", "MEMORY.md"), { recursive: true });
|
||||
|
||||
const prompt = await buildAgentChatPrompt({
|
||||
agent: makeAgent({
|
||||
name: "Avery",
|
||||
role: "reviewer",
|
||||
}),
|
||||
rootDir: testDir,
|
||||
basePrompt: "You are a chat assistant.",
|
||||
includeProjectMemory: true,
|
||||
});
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("agent-instructions");
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("Failed to read project memory");
|
||||
expect(prompt).toContain("## Identity");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,6 +6,9 @@ import {
|
||||
type AgentRatingSummary,
|
||||
type AgentStore,
|
||||
} from "@fusion/core";
|
||||
import { createLogger } from "./logger.js";
|
||||
|
||||
const log = createLogger("agent-instructions");
|
||||
|
||||
const MAX_INSTRUCTIONS_PATH_LENGTH = 500;
|
||||
const MAX_INSTRUCTIONS_TEXT_LENGTH = 50_000;
|
||||
@@ -21,9 +24,7 @@ function trimAndClamp(value: string, maxLength: number, label: string, agentId:
|
||||
return trimmed;
|
||||
}
|
||||
|
||||
console.warn(
|
||||
`[agent-instructions] ${label} exceeded max length for agent ${agentId}; truncating to ${maxLength} chars`,
|
||||
);
|
||||
log.warn(`${label} exceeded max length for agent ${agentId}; truncating to ${maxLength} chars`);
|
||||
return trimmed.slice(0, maxLength);
|
||||
}
|
||||
|
||||
@@ -38,32 +39,32 @@ function resolveValidatedInstructionsPath(rawPath: string, rootDir: string, agen
|
||||
}
|
||||
|
||||
if (trimmed.length > MAX_INSTRUCTIONS_PATH_LENGTH) {
|
||||
console.warn(
|
||||
`[agent-instructions] instructionsPath too long for agent ${agentId} (${trimmed.length} > ${MAX_INSTRUCTIONS_PATH_LENGTH})`,
|
||||
log.warn(
|
||||
`instructionsPath too long for agent ${agentId} (${trimmed.length} > ${MAX_INSTRUCTIONS_PATH_LENGTH})`,
|
||||
);
|
||||
return null;
|
||||
}
|
||||
|
||||
if (!trimmed.toLowerCase().endsWith(".md")) {
|
||||
console.warn(`[agent-instructions] instructionsPath must end in .md for agent ${agentId}: ${trimmed}`);
|
||||
log.warn(`instructionsPath must end in .md for agent ${agentId}: ${trimmed}`);
|
||||
return null;
|
||||
}
|
||||
|
||||
if (isAbsolute(trimmed)) {
|
||||
console.warn(`[agent-instructions] instructionsPath must be project-relative for agent ${agentId}: ${trimmed}`);
|
||||
log.warn(`instructionsPath must be project-relative for agent ${agentId}: ${trimmed}`);
|
||||
return null;
|
||||
}
|
||||
|
||||
const normalized = normalize(trimmed);
|
||||
if (isPathTraversal(normalized)) {
|
||||
console.warn(`[agent-instructions] instructionsPath traversal is not allowed for agent ${agentId}: ${trimmed}`);
|
||||
log.warn(`instructionsPath traversal is not allowed for agent ${agentId}: ${trimmed}`);
|
||||
return null;
|
||||
}
|
||||
|
||||
const resolvedPath = resolve(rootDir, normalized);
|
||||
const rel = relative(rootDir, resolvedPath);
|
||||
if (!rel || rel.startsWith(`..${sep}`) || rel === ".." || isAbsolute(rel)) {
|
||||
console.warn(`[agent-instructions] instructionsPath escapes project root for agent ${agentId}: ${trimmed}`);
|
||||
log.warn(`instructionsPath escapes project root for agent ${agentId}: ${trimmed}`);
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -187,13 +188,9 @@ export async function resolveAgentInstructions(
|
||||
// Log a warning but don't throw — instructionsText is still used
|
||||
const code = (err as NodeJS.ErrnoException).code;
|
||||
if (code === "ENOENT") {
|
||||
console.warn(
|
||||
`[agent-instructions] Instructions file not found for agent ${agent.id}: ${filePath}`,
|
||||
);
|
||||
log.warn(`Instructions file not found for agent ${agent.id}: ${filePath}`);
|
||||
} else {
|
||||
console.warn(
|
||||
`[agent-instructions] Failed to read instructions file for agent ${agent.id}: ${filePath} (${code})`,
|
||||
);
|
||||
log.warn(`Failed to read instructions file for agent ${agent.id}: ${filePath} (${code})`);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -277,7 +274,7 @@ export async function buildAgentChatPrompt(options: {
|
||||
// Graceful fallback for chat/heartbeat: if project memory cannot be read,
|
||||
// continue with available identity + agent instructions.
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
console.warn(`[agent-instructions] Failed to read project memory for agent ${agent.id}: ${message}`);
|
||||
log.warn(`Failed to read project memory for agent ${agent.id}: ${message}`);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user