fix: avoid qqbot session file key collisions

This commit is contained in:
Tak Hoffman
2026-04-10 19:40:18 -05:00
parent 24ac5ddf7f
commit 9ec0dc7ac5
4 changed files with 180 additions and 14 deletions
@@ -0,0 +1,94 @@
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { afterEach, describe, expect, it, vi } from "vitest";
import type { SessionState } from "./session-store.js";
type SessionStoreModule = typeof import("./session-store.js");
async function loadSessionStore(testRoot: string): Promise<SessionStoreModule> {
vi.resetModules();
vi.doMock("./utils/platform.js", () => ({
getQQBotDataDir: (...subPaths: string[]) => {
const dir = path.join(testRoot, ...subPaths);
fs.mkdirSync(dir, { recursive: true });
return dir;
},
}));
return import("./session-store.js");
}
function buildSession(accountId: string, overrides: Partial<SessionState> = {}): SessionState {
return {
accountId,
intentLevelIndex: 0,
lastConnectedAt: 1_700_000_000_000,
lastSeq: 42,
savedAt: 1_700_000_000_000,
sessionId: `session-${accountId}`,
...overrides,
};
}
describe("qqbot session store", () => {
const tempRoots: string[] = [];
afterEach(() => {
vi.resetModules();
vi.doUnmock("./utils/platform.js");
vi.restoreAllMocks();
for (const root of tempRoots.splice(0)) {
fs.rmSync(root, { recursive: true, force: true });
}
});
it("keeps distinct sessions when account ids collide under the legacy filename sanitizer", async () => {
const testRoot = fs.mkdtempSync(path.join(os.tmpdir(), "qqbot-session-store-"));
tempRoots.push(testRoot);
const store = await loadSessionStore(testRoot);
const colonAccount = "acct:one";
const slashAccount = "acct/one";
store.saveSession(buildSession(colonAccount, { lastSeq: 11, sessionId: "colon-session" }));
store.saveSession(buildSession(slashAccount, { lastSeq: 22, sessionId: "slash-session" }));
expect(store.loadSession(colonAccount)).toMatchObject({
accountId: colonAccount,
lastSeq: 11,
sessionId: "colon-session",
});
expect(store.loadSession(slashAccount)).toMatchObject({
accountId: slashAccount,
lastSeq: 22,
sessionId: "slash-session",
});
const sessionFiles = fs
.readdirSync(path.join(testRoot, "sessions"))
.filter((file) => file.startsWith("session-") && file.endsWith(".json"));
expect(sessionFiles).toHaveLength(2);
});
it("loads a legacy sanitized session file for backward compatibility", async () => {
const testRoot = fs.mkdtempSync(path.join(os.tmpdir(), "qqbot-session-store-"));
tempRoots.push(testRoot);
const store = await loadSessionStore(testRoot);
const accountId = "legacy/account:id";
const legacyPath = path.join(
testRoot,
"sessions",
`session-${accountId.replace(/[^a-zA-Z0-9_-]/g, "_")}.json`,
);
fs.mkdirSync(path.dirname(legacyPath), { recursive: true });
fs.writeFileSync(
legacyPath,
JSON.stringify(buildSession(accountId, { savedAt: Date.now(), sessionId: "legacy" })),
);
expect(store.loadSession(accountId)).toMatchObject({
accountId,
sessionId: "legacy",
});
});
});
+47 -12
View File
@@ -35,18 +35,39 @@ function ensureDir(): void {
}
}
/** Return the session file path for one account. */
function getSessionPath(accountId: string): string {
function encodeAccountIdForFileName(accountId: string): string {
return Buffer.from(accountId, "utf8").toString("base64url");
}
function getLegacySessionPath(accountId: string): string {
const safeId = accountId.replace(/[^a-zA-Z0-9_-]/g, "_");
return path.join(SESSION_DIR, `session-${safeId}.json`);
}
/** Return the session file path for one account. */
function getSessionPath(accountId: string): string {
const encodedId = encodeAccountIdForFileName(accountId);
return path.join(SESSION_DIR, `session-${encodedId}.json`);
}
function getCandidateSessionPaths(accountId: string): string[] {
const primaryPath = getSessionPath(accountId);
const legacyPath = getLegacySessionPath(accountId);
return primaryPath === legacyPath ? [primaryPath] : [primaryPath, legacyPath];
}
/** Load a saved session, rejecting expired or mismatched appId entries. */
export function loadSession(accountId: string, expectedAppId?: string): SessionState | null {
const filePath = getSessionPath(accountId);
try {
if (!fs.existsSync(filePath)) {
let filePath: string | null = null;
for (const candidatePath of getCandidateSessionPaths(accountId)) {
if (fs.existsSync(candidatePath)) {
filePath = candidatePath;
break;
}
}
if (!filePath) {
return null;
}
@@ -138,6 +159,7 @@ export function saveSession(state: SessionState): void {
/** Write one session file to disk immediately. */
function doSaveSession(state: SessionState): void {
const filePath = getSessionPath(state.accountId);
const legacyPath = getLegacySessionPath(state.accountId);
try {
ensureDir();
@@ -148,6 +170,9 @@ function doSaveSession(state: SessionState): void {
};
fs.writeFileSync(filePath, JSON.stringify(stateToSave, null, 2), "utf-8");
if (legacyPath !== filePath && fs.existsSync(legacyPath)) {
fs.unlinkSync(legacyPath);
}
debugLog(
`[session-store] Saved session for ${state.accountId}: sessionId=${state.sessionId}, lastSeq=${state.lastSeq}`,
);
@@ -158,8 +183,6 @@ function doSaveSession(state: SessionState): void {
/** Clear a saved session and any pending throttle state. */
export function clearSession(accountId: string): void {
const filePath = getSessionPath(accountId);
const throttle = throttleState.get(accountId);
if (throttle) {
if (throttle.throttleTimer) {
@@ -169,8 +192,14 @@ export function clearSession(accountId: string): void {
}
try {
if (fs.existsSync(filePath)) {
fs.unlinkSync(filePath);
let cleared = false;
for (const filePath of getCandidateSessionPaths(accountId)) {
if (fs.existsSync(filePath)) {
fs.unlinkSync(filePath);
cleared = true;
}
}
if (cleared) {
debugLog(`[session-store] Cleared session for ${accountId}`);
}
} catch (err) {
@@ -191,7 +220,7 @@ export function updateLastSeq(accountId: string, lastSeq: number): void {
/** Load all saved sessions from disk. */
export function getAllSessions(): SessionState[] {
const sessions: SessionState[] = [];
const sessions = new Map<string, SessionState>();
try {
ensureDir();
@@ -203,7 +232,13 @@ export function getAllSessions(): SessionState[] {
try {
const data = fs.readFileSync(filePath, "utf-8");
const state = JSON.parse(data) as SessionState;
sessions.push(state);
if (typeof state.accountId !== "string" || !state.accountId) {
continue;
}
const existing = sessions.get(state.accountId);
if (!existing || (state.savedAt ?? 0) >= (existing.savedAt ?? 0)) {
sessions.set(state.accountId, state);
}
} catch {
// Ignore malformed session files here.
}
@@ -213,7 +248,7 @@ export function getAllSessions(): SessionState[] {
// Ignore missing directories and similar filesystem errors.
}
return sessions;
return [...sessions.values()];
}
/**
@@ -5,7 +5,7 @@ import type {
CostUsageTotals,
SessionCostSummary,
} from "../../infra/session-cost-usage.js";
import { handleUsageCommand } from "./commands-session.js";
import { handleFastCommand, handleUsageCommand } from "./commands-session.js";
import type { HandleCommandsParams } from "./commands-types.js";
const resolveSessionAgentIdMock = vi.hoisted(() => vi.fn(() => "main"));
@@ -32,6 +32,9 @@ const loadCostUsageSummaryMock = vi.hoisted(() =>
},
})),
);
const resolveFastModeStateMock = vi.hoisted(() =>
vi.fn(() => ({ enabled: true, source: "agent" })),
);
vi.mock("../../agents/agent-scope.js", async () => {
const actual = await vi.importActual<typeof import("../../agents/agent-scope.js")>(
@@ -48,6 +51,10 @@ vi.mock("../../infra/session-cost-usage.js", () => ({
loadCostUsageSummary: loadCostUsageSummaryMock,
}));
vi.mock("../../agents/fast-mode.js", () => ({
resolveFastModeState: resolveFastModeStateMock,
}));
function buildUsageParams(): HandleCommandsParams {
return {
cfg: {
@@ -131,3 +138,30 @@ describe("handleUsageCommand", () => {
);
});
});
describe("handleFastCommand", () => {
beforeEach(() => {
vi.clearAllMocks();
resolveSessionAgentIdMock.mockReturnValue("target");
resolveFastModeStateMock.mockReturnValue({ enabled: true, source: "agent" });
});
it("uses the canonical target session agent for /fast status", async () => {
const params = buildUsageParams();
params.command.commandBodyNormalized = "/fast status";
params.provider = "openai";
params.model = "gpt-5.4";
const result = await handleFastCommand(params, true);
expect(result?.shouldContinue).toBe(false);
expect(resolveFastModeStateMock).toHaveBeenCalledWith(
expect.objectContaining({
agentId: "target",
provider: "openai",
model: "gpt-5.4",
}),
);
expect(result?.reply?.text).toContain("Current fast mode: on");
});
});
+4 -1
View File
@@ -354,11 +354,14 @@ export const handleFastCommand: CommandHandler = async (params, allowTextCommand
const rawArgs = normalized === "/fast" ? "" : normalized.slice("/fast".length).trim();
const rawMode = normalizeLowercaseStringOrEmpty(rawArgs);
if (!rawMode || rawMode === "status") {
const sessionAgentId = params.sessionKey
? resolveSessionAgentId({ sessionKey: params.sessionKey, config: params.cfg })
: params.agentId;
const state = resolveFastModeState({
cfg: params.cfg,
provider: params.provider,
model: params.model,
agentId: params.agentId,
agentId: sessionAgentId,
sessionEntry: params.sessionEntry,
});
const suffix =