Skip to content

Commit c3237a3

Browse files
committed
fix: move assignMessageRefs after prune to prevent orphan alias leak
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.
1 parent 3dec9f2 commit c3237a3

2 files changed

Lines changed: 143 additions & 1 deletion

File tree

lib/hooks.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,11 @@ export function createChatMessageTransformHandler(
124124

125125
stripHallucinations(output.messages)
126126
cacheSystemPromptTokens(state, output.messages)
127-
assignMessageRefs(state, output.messages)
128127
syncCompressionBlocks(state, logger, output.messages)
129128
syncToolCache(state, config, logger, output.messages)
130129
buildToolIdList(state, output.messages)
131130
prune(state, logger, config, output.messages)
131+
assignMessageRefs(state, output.messages)
132132
await injectExtendedSubAgentResults(
133133
client,
134134
state,

tests/message-ids.test.ts

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,42 @@
11
import assert from "node:assert/strict"
22
import test from "node:test"
3+
import type { PluginConfig } from "../lib/config"
4+
import { createChatMessageTransformHandler } from "../lib/hooks"
35
import { Logger } from "../lib/logger"
46
import { assignMessageRefs } from "../lib/message-ids"
57
import { checkSession, createSessionState, type WithParts } from "../lib/state"
68

9+
function buildConfig(permission: "allow" | "ask" | "deny" = "allow"): PluginConfig {
10+
return {
11+
enabled: true,
12+
debug: false,
13+
pruneNotification: "off",
14+
pruneNotificationType: "chat",
15+
commands: { enabled: true, protectedTools: [] },
16+
manualMode: { enabled: false, automaticStrategies: true },
17+
turnProtection: { enabled: false, turns: 4 },
18+
experimental: { allowSubAgents: false, customPrompts: false },
19+
protectedFilePatterns: [],
20+
compress: {
21+
mode: "message",
22+
permission,
23+
showCompression: false,
24+
maxContextLimit: 150000,
25+
minContextLimit: 50000,
26+
nudgeFrequency: 5,
27+
iterationNudgeThreshold: 15,
28+
nudgeForce: "soft",
29+
protectedTools: ["task"],
30+
protectTags: false,
31+
protectUserMessages: false,
32+
},
33+
strategies: {
34+
deduplication: { enabled: true, protectedTools: [] },
35+
purgeErrors: { enabled: true, turns: 4, protectedTools: [] },
36+
},
37+
}
38+
}
39+
740
function textPart(messageID: string, sessionID: string, id: string, text: string) {
841
return {
942
id,
@@ -87,3 +120,112 @@ test("checkSession resets message id aliases after native compaction", async ()
87120
assert.equal(state.messageIds.byRef.get("m0002"), "msg-user-follow-up")
88121
assert.equal(state.messageIds.nextRef, 3)
89122
})
123+
124+
test(
125+
"assignMessageRefs after prune prevents orphan aliases (RED->GREEN: fails on old pipeline order)",
126+
async () => {
127+
const sessionID = "ses_alias_leak_test"
128+
const state = createSessionState()
129+
const logger = new Logger(false)
130+
131+
// Prevent checkSession from resetting state via ensureSessionInitialized
132+
state.sessionId = sessionID
133+
134+
// Simulate a completed compress — this message should be removed by prune
135+
state.prune.messages.byMessageId.set("msg-pruned", {
136+
tokenCount: 100,
137+
allBlockIds: [1],
138+
activeBlockIds: [1],
139+
})
140+
141+
const messages: WithParts[] = [
142+
{
143+
info: {
144+
id: "msg-user",
145+
role: "user",
146+
sessionID,
147+
agent: "assistant",
148+
model: { providerID: "anthropic", modelID: "claude-test" },
149+
time: { created: 1 },
150+
} as WithParts["info"],
151+
parts: [
152+
{
153+
id: "msg-user-part",
154+
messageID: "msg-user",
155+
sessionID,
156+
type: "text",
157+
text: "Test message",
158+
},
159+
],
160+
},
161+
{
162+
info: {
163+
id: "msg-assistant-survivor",
164+
role: "assistant",
165+
sessionID,
166+
agent: "assistant",
167+
time: { created: 2 },
168+
} as WithParts["info"],
169+
parts: [
170+
{
171+
id: "msg-assistant-survivor-part",
172+
messageID: "msg-assistant-survivor",
173+
sessionID,
174+
type: "text",
175+
text: "Survivor response",
176+
},
177+
],
178+
},
179+
{
180+
info: {
181+
id: "msg-pruned",
182+
role: "assistant",
183+
sessionID,
184+
agent: "assistant",
185+
time: { created: 3 },
186+
} as WithParts["info"],
187+
parts: [
188+
{
189+
id: "msg-pruned-part",
190+
messageID: "msg-pruned",
191+
sessionID,
192+
type: "text",
193+
text: "Will be removed",
194+
},
195+
],
196+
},
197+
]
198+
199+
const handler = createChatMessageTransformHandler(
200+
{} as any,
201+
state,
202+
logger,
203+
buildConfig("allow"),
204+
{
205+
reload() {},
206+
getRuntimePrompts() {
207+
return {} as any
208+
},
209+
} as any,
210+
{ global: undefined, agents: {} },
211+
)
212+
213+
await handler({}, { messages })
214+
215+
// Survivors get aliases
216+
assert.ok(state.messageIds.byRawId.has("msg-user"), "user message should have alias")
217+
assert.ok(
218+
state.messageIds.byRawId.has("msg-assistant-survivor"),
219+
"survivor should have alias",
220+
)
221+
222+
// RED on old pipeline (assignMessageRefs before prune): msg-pruned got an alias before
223+
// being removed, and no cleanup follows → assertion FAILS (alias leak detected)
224+
// GREEN on new pipeline (assignMessageRefs after prune): prune removes msg-pruned first,
225+
// then assignMessageRefs never sees it → assertion PASSES (no leak)
226+
assert.ok(
227+
!state.messageIds.byRawId.has("msg-pruned"),
228+
"pruned message should NOT have an alias — leak if it does",
229+
)
230+
},
231+
)

0 commit comments

Comments
 (0)