From 2cc6b8825eaadc47b8a61e01a098cc2213397f4d Mon Sep 17 00:00:00 2001 From: zhexulong <112891841+zhexulong@users.noreply.github.com> Date: Wed, 17 Jun 2026 01:24:54 +0800 Subject: [PATCH 1/3] fix: move assignMessageRefs after prune to prevent orphan alias leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In createChatMessageTransformHandler, assignMessageRefs ran before prune. Every message got an mXXXX alias, but then prune removed some messages — those orphan aliases accumulated until the 9999-ref limit was hit, crashing the session. Moving assignMessageRefs after prune means only survivors get aliases. No leak, no capacity exhaustion. Also adds a RED->GREEN integration test that exercises the full pipeline through createChatMessageTransformHandler. It FAILS on the old ordering (proving the leak exists) and PASSes on the fixed ordering. --- lib/hooks.ts | 2 +- tests/message-ids.test.ts | 136 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 1 deletion(-) diff --git a/lib/hooks.ts b/lib/hooks.ts index 67030f1c..6192f3a0 100644 --- a/lib/hooks.ts +++ b/lib/hooks.ts @@ -124,11 +124,11 @@ export function createChatMessageTransformHandler( stripHallucinations(output.messages) cacheSystemPromptTokens(state, output.messages) - assignMessageRefs(state, output.messages) syncCompressionBlocks(state, logger, output.messages) syncToolCache(state, config, logger, output.messages) buildToolIdList(state, output.messages) prune(state, logger, config, output.messages) + assignMessageRefs(state, output.messages) await injectExtendedSubAgentResults( client, state, diff --git a/tests/message-ids.test.ts b/tests/message-ids.test.ts index f128b766..d987b50d 100644 --- a/tests/message-ids.test.ts +++ b/tests/message-ids.test.ts @@ -1,9 +1,42 @@ import assert from "node:assert/strict" import test from "node:test" +import type { PluginConfig } from "../lib/config" +import { createChatMessageTransformHandler } from "../lib/hooks" import { Logger } from "../lib/logger" import { assignMessageRefs } from "../lib/message-ids" import { checkSession, createSessionState, type WithParts } from "../lib/state" +function buildConfig(permission: "allow" | "ask" | "deny" = "allow"): PluginConfig { + return { + enabled: true, + debug: false, + pruneNotification: "off", + pruneNotificationType: "chat", + commands: { enabled: true, protectedTools: [] }, + manualMode: { enabled: false, automaticStrategies: true }, + turnProtection: { enabled: false, turns: 4 }, + experimental: { allowSubAgents: false, customPrompts: false }, + protectedFilePatterns: [], + compress: { + mode: "message", + permission, + showCompression: false, + maxContextLimit: 150000, + minContextLimit: 50000, + nudgeFrequency: 5, + iterationNudgeThreshold: 15, + nudgeForce: "soft", + protectedTools: ["task"], + protectTags: false, + protectUserMessages: false, + }, + strategies: { + deduplication: { enabled: true, protectedTools: [] }, + purgeErrors: { enabled: true, turns: 4, protectedTools: [] }, + }, + } +} + function textPart(messageID: string, sessionID: string, id: string, text: string) { return { id, @@ -87,3 +120,106 @@ test("checkSession resets message id aliases after native compaction", async () assert.equal(state.messageIds.byRef.get("m0002"), "msg-user-follow-up") assert.equal(state.messageIds.nextRef, 3) }) + +test("assignMessageRefs after prune prevents orphan aliases (RED->GREEN: fails on old pipeline order)", async () => { + const sessionID = "ses_alias_leak_test" + const state = createSessionState() + const logger = new Logger(false) + + // Prevent checkSession from resetting state via ensureSessionInitialized + state.sessionId = sessionID + + // Simulate a completed compress — this message should be removed by prune + state.prune.messages.byMessageId.set("msg-pruned", { + tokenCount: 100, + allBlockIds: [1], + activeBlockIds: [1], + }) + + const messages: WithParts[] = [ + { + info: { + id: "msg-user", + role: "user", + sessionID, + agent: "assistant", + model: { providerID: "anthropic", modelID: "claude-test" }, + time: { created: 1 }, + } as WithParts["info"], + parts: [ + { + id: "msg-user-part", + messageID: "msg-user", + sessionID, + type: "text", + text: "Test message", + }, + ], + }, + { + info: { + id: "msg-assistant-survivor", + role: "assistant", + sessionID, + agent: "assistant", + time: { created: 2 }, + } as WithParts["info"], + parts: [ + { + id: "msg-assistant-survivor-part", + messageID: "msg-assistant-survivor", + sessionID, + type: "text", + text: "Survivor response", + }, + ], + }, + { + info: { + id: "msg-pruned", + role: "assistant", + sessionID, + agent: "assistant", + time: { created: 3 }, + } as WithParts["info"], + parts: [ + { + id: "msg-pruned-part", + messageID: "msg-pruned", + sessionID, + type: "text", + text: "Will be removed", + }, + ], + }, + ] + + const handler = createChatMessageTransformHandler( + {} as any, + state, + logger, + buildConfig("allow"), + { + reload() {}, + getRuntimePrompts() { + return {} as any + }, + } as any, + { global: undefined, agents: {} }, + ) + + await handler({}, { messages }) + + // Survivors get aliases + assert.ok(state.messageIds.byRawId.has("msg-user"), "user message should have alias") + assert.ok(state.messageIds.byRawId.has("msg-assistant-survivor"), "survivor should have alias") + + // RED on old pipeline (assignMessageRefs before prune): msg-pruned got an alias before + // being removed, and no cleanup follows → assertion FAILS (alias leak detected) + // GREEN on new pipeline (assignMessageRefs after prune): prune removes msg-pruned first, + // then assignMessageRefs never sees it → assertion PASSES (no leak) + assert.ok( + !state.messageIds.byRawId.has("msg-pruned"), + "pruned message should NOT have an alias — leak if it does", + ) +}) From fe61437f4917d36de2b3f3bb80de802eaf5b2f53 Mon Sep 17 00:00:00 2001 From: zhexulong <112891841+zhexulong@users.noreply.github.com> Date: Thu, 18 Jun 2026 10:17:40 +0800 Subject: [PATCH 2/3] fix(update): prevent updateRemoveDir from deleting non-wrapper directories The ?? fallback after wrapperSpec() returned undefined was reading dependencies from an arbitrary parent directory's package.json, which could match and return that directory as the removal target. For example, when the plugin sits inside .config/opencode/node_modules/, updateRemoveDir would return .config/opencode as the dir to rm -rf, because .config/opencode/package.json lists the DCP dependency. Remove the fallback entirely. wrapperSpec() is the sole authority: if the wrapper directory doesn't match the expected opencode naming convention (@scope/pkg@spec), return undefined. Add a regression test that reproduces the .config/opencode-like directory structure and asserts undefined is returned. --- lib/update.ts | 3 +-- tests/update.test.ts | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/update.ts b/lib/update.ts index 89715957..c42035d3 100644 --- a/lib/update.ts +++ b/lib/update.ts @@ -87,8 +87,7 @@ export async function updateRemoveDir(packageDir: string, name: string) { if (basename(nodeModulesDir) !== "node_modules") return undefined const wrapperDir = dirname(nodeModulesDir) - const wrapperPkg = await readPackageJson(join(wrapperDir, "package.json")) - const spec = wrapperSpec(wrapperDir, name) ?? wrapperPkg?.dependencies?.[name] + const spec = wrapperSpec(wrapperDir, name) if (!spec || !isAutoUpdatableSpec(spec)) return undefined return wrapperDir diff --git a/tests/update.test.ts b/tests/update.test.ts index f3f67168..64def1a9 100644 --- a/tests/update.test.ts +++ b/tests/update.test.ts @@ -40,6 +40,23 @@ test("updateRemoveDir removes opencode npm wrapper for latest installs", async ( assert.equal(await updateRemoveDir(packageDir, "@tarquinen/opencode-dcp"), wrapperDir) }) +test("updateRemoveDir rejects directories not matching opencode wrapper naming", async () => { + const rootDir = await mkdtemp(join(tmpdir(), "dcp-update-")) + // Simulate .config/opencode-like setup: plugin lives in node_modules but + // the parent dir is NOT an @scope/pkg@spec wrapper — updateRemoveDir + // must NOT return the parent dir even if its package.json lists the dep. + const packageDir = join(rootDir, "node_modules", "@tarquinen", "opencode-dcp") + await writePackageJson(rootDir, { + dependencies: { "@tarquinen/opencode-dcp": "latest" }, + }) + await writePackageJson(packageDir, { + name: "@tarquinen/opencode-dcp", + version: "3.1.9", + }) + + assert.equal(await updateRemoveDir(packageDir, "@tarquinen/opencode-dcp"), undefined) +}) + test("updateRemoveDir skips version-locked opencode installs", async () => { const rootDir = await mkdtemp(join(tmpdir(), "dcp-update-")) const wrapperDir = join(rootDir, "@tarquinen", "opencode-dcp@3.1.9") From e81aa3944b411720658ba49a93daeb0133eb488b Mon Sep 17 00:00:00 2001 From: zhexulong <112891841+zhexulong@users.noreply.github.com> Date: Thu, 18 Jun 2026 11:09:49 +0800 Subject: [PATCH 3/3] Revert "fix(update): prevent updateRemoveDir from deleting non-wrapper directories" This reverts commit fe61437f4917d36de2b3f3bb80de802eaf5b2f53. --- lib/update.ts | 3 ++- tests/update.test.ts | 17 ----------------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/lib/update.ts b/lib/update.ts index c42035d3..89715957 100644 --- a/lib/update.ts +++ b/lib/update.ts @@ -87,7 +87,8 @@ export async function updateRemoveDir(packageDir: string, name: string) { if (basename(nodeModulesDir) !== "node_modules") return undefined const wrapperDir = dirname(nodeModulesDir) - const spec = wrapperSpec(wrapperDir, name) + const wrapperPkg = await readPackageJson(join(wrapperDir, "package.json")) + const spec = wrapperSpec(wrapperDir, name) ?? wrapperPkg?.dependencies?.[name] if (!spec || !isAutoUpdatableSpec(spec)) return undefined return wrapperDir diff --git a/tests/update.test.ts b/tests/update.test.ts index 64def1a9..f3f67168 100644 --- a/tests/update.test.ts +++ b/tests/update.test.ts @@ -40,23 +40,6 @@ test("updateRemoveDir removes opencode npm wrapper for latest installs", async ( assert.equal(await updateRemoveDir(packageDir, "@tarquinen/opencode-dcp"), wrapperDir) }) -test("updateRemoveDir rejects directories not matching opencode wrapper naming", async () => { - const rootDir = await mkdtemp(join(tmpdir(), "dcp-update-")) - // Simulate .config/opencode-like setup: plugin lives in node_modules but - // the parent dir is NOT an @scope/pkg@spec wrapper — updateRemoveDir - // must NOT return the parent dir even if its package.json lists the dep. - const packageDir = join(rootDir, "node_modules", "@tarquinen", "opencode-dcp") - await writePackageJson(rootDir, { - dependencies: { "@tarquinen/opencode-dcp": "latest" }, - }) - await writePackageJson(packageDir, { - name: "@tarquinen/opencode-dcp", - version: "3.1.9", - }) - - assert.equal(await updateRemoveDir(packageDir, "@tarquinen/opencode-dcp"), undefined) -}) - test("updateRemoveDir skips version-locked opencode installs", async () => { const rootDir = await mkdtemp(join(tmpdir(), "dcp-update-")) const wrapperDir = join(rootDir, "@tarquinen", "opencode-dcp@3.1.9")