fix teams feedback learning filename collisions

This commit is contained in:
Tak Hoffman
2026-04-10 18:57:06 -05:00
parent f2d9b9c69c
commit a9100a33c2
2 changed files with 97 additions and 28 deletions

View File

@@ -7,10 +7,36 @@ const lastReflectionBySession = new Map<string, number>();
/** Maximum cooldown entries before pruning expired ones. */
const MAX_COOLDOWN_ENTRIES = 500;
function sanitizeSessionKey(sessionKey: string): string {
function legacySanitizeSessionKey(sessionKey: string): string {
return sessionKey.replace(/[^a-zA-Z0-9_-]/g, "_");
}
function encodeSessionKey(sessionKey: string): string {
return Buffer.from(sessionKey, "utf8").toString("base64url");
}
function resolveLearningsFilePath(storePath: string, sessionKey: string): string {
return `${storePath}/${encodeSessionKey(sessionKey)}.learnings.json`;
}
function resolveLegacyLearningsFilePath(storePath: string, sessionKey: string): string {
return `${storePath}/${legacySanitizeSessionKey(sessionKey)}.learnings.json`;
}
async function readLearningsFile(
filePath: string,
): Promise<{ exists: boolean; learnings: string[] }> {
const fs = await import("node:fs/promises");
try {
const content = await fs.readFile(filePath, "utf-8");
const parsed = JSON.parse(content);
return { exists: true, learnings: Array.isArray(parsed) ? parsed : [] };
} catch {
return { exists: false, learnings: [] };
}
}
/** Prune expired cooldown entries to prevent unbounded memory growth. */
function pruneExpiredCooldowns(cooldownMs: number): void {
if (lastReflectionBySession.size <= MAX_COOLDOWN_ENTRIES) {
@@ -54,21 +80,15 @@ export async function storeSessionLearning(params: {
const fs = await import("node:fs/promises");
const path = await import("node:path");
const learningsFile = path.join(
params.storePath,
`${sanitizeSessionKey(params.sessionKey)}.learnings.json`,
);
const learningsFile = resolveLearningsFilePath(params.storePath, params.sessionKey);
const legacyLearningsFile = resolveLegacyLearningsFilePath(params.storePath, params.sessionKey);
const { exists, learnings: existingLearnings } = await readLearningsFile(learningsFile);
const { learnings: legacyLearnings } =
exists || legacyLearningsFile === learningsFile
? { learnings: [] as string[] }
: await readLearningsFile(legacyLearningsFile);
let learnings: string[] = [];
try {
const existing = await fs.readFile(learningsFile, "utf-8");
const parsed = JSON.parse(existing);
if (Array.isArray(parsed)) {
learnings = parsed;
}
} catch {
// File doesn't exist yet — start fresh.
}
let learnings = exists ? existingLearnings : legacyLearnings;
learnings.push(params.learning);
if (learnings.length > 10) {
@@ -77,6 +97,9 @@ export async function storeSessionLearning(params: {
await fs.mkdir(path.dirname(learningsFile), { recursive: true });
await fs.writeFile(learningsFile, JSON.stringify(learnings, null, 2), "utf-8");
if (!exists && legacyLearningsFile !== learningsFile) {
await fs.rm(legacyLearningsFile, { force: true }).catch(() => undefined);
}
}
/** Load session learnings for injection into extraSystemPrompt. */
@@ -84,16 +107,10 @@ export async function loadSessionLearnings(
storePath: string,
sessionKey: string,
): Promise<string[]> {
const fs = await import("node:fs/promises");
const path = await import("node:path");
const learningsFile = path.join(storePath, `${sanitizeSessionKey(sessionKey)}.learnings.json`);
try {
const content = await fs.readFile(learningsFile, "utf-8");
const parsed = JSON.parse(content);
return Array.isArray(parsed) ? parsed : [];
} catch {
return [];
const learningsFile = resolveLearningsFilePath(storePath, sessionKey);
const { exists, learnings } = await readLearningsFile(learningsFile);
if (exists) {
return learnings;
}
return (await readLearningsFile(resolveLegacyLearningsFilePath(storePath, sessionKey))).learnings;
}

View File

@@ -2,6 +2,7 @@ import { mkdtemp, rm, writeFile } from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { afterEach, describe, expect, it, vi } from "vitest";
import { storeSessionLearning } from "./feedback-reflection-store.js";
import {
buildFeedbackEvent,
buildReflectionPrompt,
@@ -174,12 +175,63 @@ describe("loadSessionLearnings", () => {
it("reads existing learnings", async () => {
tmpDir = await mkdtemp(path.join(os.tmpdir(), "learnings-test-"));
// Colons are sanitized to underscores in filenames (Windows compat)
const safeKey = "msteams_user1";
const safeKey = Buffer.from("msteams:user1", "utf8").toString("base64url");
const filePath = path.join(tmpDir, `${safeKey}.learnings.json`);
await writeFile(filePath, JSON.stringify(["Be concise", "Use examples"]), "utf-8");
const learnings = await loadSessionLearnings(tmpDir, "msteams:user1");
expect(learnings).toEqual(["Be concise", "Use examples"]);
});
it("keeps distinct session keys isolated across the filename persistence boundary", async () => {
tmpDir = await mkdtemp(path.join(os.tmpdir(), "learnings-test-"));
await storeSessionLearning({
storePath: tmpDir,
sessionKey: "msteams:user1",
learning: "Use bullets",
});
await storeSessionLearning({
storePath: tmpDir,
sessionKey: "msteams/user1",
learning: "Avoid bullets",
});
await expect(loadSessionLearnings(tmpDir, "msteams:user1")).resolves.toEqual(["Use bullets"]);
await expect(loadSessionLearnings(tmpDir, "msteams/user1")).resolves.toEqual(["Avoid bullets"]);
});
it("reads and migrates legacy sanitized session learning files", async () => {
tmpDir = await mkdtemp(path.join(os.tmpdir(), "learnings-test-"));
const legacyFile = path.join(tmpDir, "msteams_user1.learnings.json");
await writeFile(legacyFile, JSON.stringify(["Legacy learning"]), "utf-8");
await expect(loadSessionLearnings(tmpDir, "msteams:user1")).resolves.toEqual([
"Legacy learning",
]);
await storeSessionLearning({
storePath: tmpDir,
sessionKey: "msteams:user1",
learning: "New learning",
});
const migratedFile = path.join(
tmpDir,
`${Buffer.from("msteams:user1", "utf8").toString("base64url")}.learnings.json`,
);
await expect(loadSessionLearnings(tmpDir, "msteams:user1")).resolves.toEqual([
"Legacy learning",
"New learning",
]);
await expect(rm(legacyFile, { force: false })).rejects.toMatchObject({ code: "ENOENT" });
await expect(loadSessionLearnings(tmpDir, "msteams:user1")).resolves.toEqual([
"Legacy learning",
"New learning",
]);
await expect(loadSessionLearnings(tmpDir, "msteams/user1")).resolves.toEqual([]);
await expect(
import("node:fs/promises").then((fs) => fs.readFile(migratedFile, "utf-8")),
).resolves.toContain("Legacy learning");
});
});