From 8813967b3adefbbdf878764b8fd2a9201de832ae Mon Sep 17 00:00:00 2001 From: Anton Date: Mon, 28 Sep 2026 15:19:20 +0300 Subject: [PATCH] fix(state): delete the session state file when a session is deleted Closes #557 DCP persists prune state per session at $XDG_DATA_HOME/opencode/storage/plugin/dcp/{sessionId}.json. Nothing ever removed those files, so deleting a session left an orphan on disk forever - and loadAllSessionStats kept counting it, because it aggregates every file it finds in that directory. The host already emits session.deleted with the session id, and createEventHandler previously only looked at message.part.updated, so the delete branch is simply unreachable today. deleteSessionState unlinks the file and swallows the error, since the file may never have been written or may already be gone. If the deleted session is the one the plugin holds state for, the in-memory session id is detached so a later request re-initialises instead of writing the file back. The session.deleted check runs before the timestamp extraction so it cannot change the behaviour of the compression timing branch, which is covered by a test. Tests: tests/session-cleanup.test.ts, six cases. Three fail on the current implementation with `the orphaned state file survived`. --- lib/hooks.ts | 21 +++++- lib/state/persistence.ts | 9 +++ tests/session-cleanup.test.ts | 117 ++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 tests/session-cleanup.test.ts diff --git a/lib/hooks.ts b/lib/hooks.ts index 6d6f3862..1e9edc07 100644 --- a/lib/hooks.ts +++ b/lib/hooks.ts @@ -36,7 +36,13 @@ import { } from "./commands" import { type HostPermissionSnapshot } from "./host-permissions" import { compressPermission, syncCompressPermissionState } from "./compress-permission" -import { checkSession, ensureSessionInitialized, saveSessionState, syncToolCache } from "./state" +import { + checkSession, + deleteSessionState, + ensureSessionInitialized, + saveSessionState, + syncToolCache, +} from "./state" import { cacheSystemPromptTokens } from "./ui/utils" const INTERNAL_AGENT_SIGNATURES = [ @@ -300,6 +306,19 @@ export function createTextCompleteHandler() { export function createEventHandler(state: SessionState, logger: Logger) { return async (input: { event: any }) => { + // Deleting a session leaves its persisted prune state behind forever, + // and the orphan still counts towards the all-time stats (#557). + if (input.event?.type === "session.deleted") { + const deletedSessionId = input.event.properties?.info?.id + if (typeof deletedSessionId === "string" && deletedSessionId.length > 0) { + await deleteSessionState(deletedSessionId, logger) + if (state.sessionId === deletedSessionId) { + state.sessionId = null + } + } + return + } + const eventTime = typeof input.event?.time === "number" && Number.isFinite(input.event.time) ? input.event.time diff --git a/lib/state/persistence.ts b/lib/state/persistence.ts index 882e7dad..92600afe 100644 --- a/lib/state/persistence.ts +++ b/lib/state/persistence.ts @@ -252,6 +252,15 @@ export async function saveManualModeSetting( await writePersistedSessionState(sessionId, state, logger) } +export async function deleteSessionState(sessionId: string, logger: Logger): Promise { + try { + await fs.unlink(getSessionFilePath(sessionId)) + logger.debug("Removed state file for deleted session", { sessionId }) + } catch { + // The file may never have been written, or may already be gone. + } +} + export interface AggregatedStats { totalTokens: number totalTools: number diff --git a/tests/session-cleanup.test.ts b/tests/session-cleanup.test.ts new file mode 100644 index 00000000..25508167 --- /dev/null +++ b/tests/session-cleanup.test.ts @@ -0,0 +1,117 @@ +import assert from "node:assert/strict" +import test from "node:test" +import { existsSync, readdirSync, rmSync, writeFileSync } from "node:fs" +import { join } from "node:path" +import { mkdtempSync } from "node:fs" +import { tmpdir } from "node:os" + +const root = mkdtempSync(join(tmpdir(), "dcp-session-delete-")) +process.env.XDG_DATA_HOME = join(root, "data") +process.env.XDG_CONFIG_HOME = join(root, "config") +process.env.OPENCODE_CONFIG_DIR = join(root, "config", "opencode") + +const { Logger } = await import("../lib/logger") +const { createSessionState, saveSessionState } = await import("../lib/state") +const { createEventHandler } = await import("../lib/hooks") + +const logger = new Logger(false) +const STORAGE_DIR = join(root, "data/opencode/storage/plugin/dcp") + +test.after(() => rmSync(root, { recursive: true, force: true })) + +// A deleted session used to leave its state file on disk forever, and the orphan +// kept contributing to the all-time stats (#557). +test("deleting a session removes its persisted state file", async () => { + const sessionId = `ses_delete_${Date.now()}` + const state = createSessionState() + state.sessionId = sessionId + state.stats.totalPruneTokens = 12_345 + await saveSessionState(state, logger) + + const filePath = join(STORAGE_DIR, `${sessionId}.json`) + assert.ok(existsSync(filePath), "precondition: the state file was written") + + const handler = createEventHandler(state, logger) + await handler({ event: { type: "session.deleted", properties: { info: { id: sessionId } } } }) + + assert.equal(existsSync(filePath), false, "the orphaned state file survived") +}) + +test("deleting another session leaves the active one intact", async () => { + const keep = `ses_keep_${Date.now()}` + const drop = `ses_drop_${Date.now()}` + const state = createSessionState() + state.sessionId = keep + await saveSessionState(state, logger) + + const dropState = createSessionState() + dropState.sessionId = drop + await saveSessionState(dropState, logger) + + const handler = createEventHandler(state, logger) + await handler({ event: { type: "session.deleted", properties: { info: { id: drop } } } }) + + assert.ok(existsSync(join(STORAGE_DIR, `${keep}.json`))) + assert.equal(existsSync(join(STORAGE_DIR, `${drop}.json`)), false) + assert.equal(state.sessionId, keep) +}) + +test("deleting the active session detaches the in-memory state", async () => { + const sessionId = `ses_active_${Date.now()}` + const state = createSessionState() + state.sessionId = sessionId + await saveSessionState(state, logger) + + const handler = createEventHandler(state, logger) + await handler({ event: { type: "session.deleted", properties: { info: { id: sessionId } } } }) + + assert.equal(state.sessionId, null) +}) + +test("a delete event for an unknown session is harmless", async () => { + const state = createSessionState() + state.sessionId = "ses_current" + const handler = createEventHandler(state, logger) + + await handler({ event: { type: "session.deleted", properties: {} } }) + await handler({ event: { type: "session.deleted" } }) + await handler({ event: { type: "session.deleted", properties: { info: { id: "" } } } }) + + assert.equal(state.sessionId, "ses_current") +}) + +test("the delete branch does not swallow the compression timing branch", async () => { + // message.part.updated must still be routed to the timing handler. + const state = createSessionState() + state.sessionId = "ses_timing" + const handler = createEventHandler(state, logger) + + await handler({ + event: { + type: "message.part.updated", + time: 1_000, + properties: { + part: { + type: "tool", + tool: "compress", + callID: "call_x", + messageID: "msg_x", + state: { status: "pending" }, + }, + }, + }, + }) + + assert.equal(state.compressionTiming.startsByCallId.size, 1) +}) + +test("unrelated events leave the storage directory alone", async () => { + const state = createSessionState() + const handler = createEventHandler(state, logger) + const before = readdirSync(STORAGE_DIR).sort() + + await handler({ event: { type: "session.updated", properties: { info: { id: "ses_x" } } } }) + await handler({ event: { type: "storage.write" } }) + + assert.deepEqual(readdirSync(STORAGE_DIR).sort(), before) +})