fix(opencode): capture current message events#2985
Closed
pixeru wants to merge 1 commit into
Closed
Conversation
1 task
Contributor
Greptile SummaryThis PR fixes the OpenCode plugin's broken capture path by replacing the stale
Confidence Score: 3/5The core mechanics work for the common streaming order (parts then finish), but the deduplication guard can produce incomplete observations when a message-complete event precedes its text parts. The new two-event capture flow introduces a deduplication guard that marks a message captured on the first text part whose trigger condition fires after a message-complete signal. Any parts arriving after that first one are silently dropped — the observation records only a fragment of the assistant reply. The convert-to-await change is a real improvement and the common ordering is unaffected, but the edge case is a silent data-loss path with no logging or retry. The capture coordination logic in Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant OC as OpenCode
participant P as Plugin (event handler)
participant M as Module-level Maps
participant W as Worker HTTP
OC->>P: "message.updated { role: assistant, id: msg1 }"
P->>M: rememberAssistantMessage(msg1, ses1)
alt isAssistantMessageComplete
P->>P: captureAssistantMessage(msg1) — no text yet, skip
P->>M: completedAssistantMessageIds.add(msg1)
end
OC->>P: "message.part.updated { messageID: msg1, text, time.end }"
P->>M: rememberTextPart(msg1, text)
P->>M: assistantMessageSessionIds.has(msg1)?
alt YES and (isTextPartComplete OR completedAssistantMessageIds)
P->>P: captureAssistantMessage(msg1)
P->>M: capturedAssistantMessageIds.add(msg1)
P->>W: POST /api/sessions/init (if new session)
P->>W: POST /api/sessions/observations
end
OC->>P: "tool.execute.after { tool, sessionID, args }"
P->>W: POST /api/sessions/init (if new session)
P->>W: POST /api/sessions/observations
OC->>P: "experimental.session.compacting { sessionID }"
P->>W: POST /api/sessions/summarize
OC->>P: "event { type: session.idle, sessionID }"
P->>W: POST /api/sessions/summarize
OC->>P: "event { type: session.deleted, sessionID }"
P->>M: Clean up all maps for sessionID
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant OC as OpenCode
participant P as Plugin (event handler)
participant M as Module-level Maps
participant W as Worker HTTP
OC->>P: "message.updated { role: assistant, id: msg1 }"
P->>M: rememberAssistantMessage(msg1, ses1)
alt isAssistantMessageComplete
P->>P: captureAssistantMessage(msg1) — no text yet, skip
P->>M: completedAssistantMessageIds.add(msg1)
end
OC->>P: "message.part.updated { messageID: msg1, text, time.end }"
P->>M: rememberTextPart(msg1, text)
P->>M: assistantMessageSessionIds.has(msg1)?
alt YES and (isTextPartComplete OR completedAssistantMessageIds)
P->>P: captureAssistantMessage(msg1)
P->>M: capturedAssistantMessageIds.add(msg1)
P->>W: POST /api/sessions/init (if new session)
P->>W: POST /api/sessions/observations
end
OC->>P: "tool.execute.after { tool, sessionID, args }"
P->>W: POST /api/sessions/init (if new session)
P->>W: POST /api/sessions/observations
OC->>P: "experimental.session.compacting { sessionID }"
P->>W: POST /api/sessions/summarize
OC->>P: "event { type: session.idle, sessionID }"
P->>W: POST /api/sessions/summarize
OC->>P: "event { type: session.deleted, sessionID }"
P->>M: Clean up all maps for sessionID
Prompt To Fix All With AIFix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
src/integrations/opencode-plugin/index.ts:324-334
**Partial capture when message completes before all parts arrive**
When `message.updated` (with `finish`) fires and `captureAssistantMessage` is called with no text yet (returns early, not marked captured), any subsequent `message.part.updated` events check `completedAssistantMessageIds.has(part.messageID)` and trigger capture on the **first** arriving part — which immediately sets `capturedAssistantMessageIds.add(messageID)`. Any parts arriving after that first one are stored in `textPartsByMessageId` but skipped in `captureAssistantMessage` because the deduplication guard fires first, producing an incomplete observation with only the first fragment of the assistant message.
### Issue 2 of 4
src/integrations/opencode-plugin/index.ts:209-218
**`textPartsByMessageId` not bounded by `pruneAssistantMessageCache`**
`rememberTextPart` writes to `textPartsByMessageId` for every `message.part.updated` event, regardless of whether the message ID is tracked in `assistantMessageSessionIds`. Because `pruneAssistantMessageCache` only evicts entries that exist in `assistantMessageSessionIds`, parts for messages that never appear in a `message.updated` event (e.g., user messages that only emit part events, or parts for abandoned streams) accumulate in `textPartsByMessageId` without bound and are never pruned.
### Issue 3 of 4
src/integrations/opencode-plugin/index.ts:309
**`info?.id` is a message ID, not a session ID**
The fallback `event?.properties?.info?.id` reads from `OpenCodeMessageInfo.id`, which is the message identifier. For `session.idle` and `session.deleted` events, this field would represent a message ID, not a session ID. If `properties.sessionID` were ever absent on those events, the wrong ID would be used as the session key — silently failing to clean up or summarize the correct session.
### Issue 4 of 4
tests/integrations/opencode-plugin-contract.test.ts:123-172
**Module-level state causes flaky assertions on repeated test runs**
`assistantMessageSessionIds`, `capturedAssistantMessageIds`, `textPartsByMessageId`, and `initializedSessionIds` are module-level singletons shared across all test cases within the same process. If the test suite is run again (e.g., Bun watch mode or a second `bun test` invocation in the same process), `ses_msg_1` will already be in `initializedSessionIds` (no init POST sent) and `msg_1` will already be in `capturedAssistantMessageIds` (no observation POST sent), causing `expect(initPost).toBeTruthy()` and `expect(obsPost).toBeTruthy()` to fail.
Reviews (1): Last reviewed commit: "fix(opencode): capture current message e..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+324
to
+334
| case "message.part.updated": { | ||
| const part = event.properties?.part; | ||
| const messageSessionID = event.properties?.sessionID || part?.sessionID; | ||
| if (!part?.messageID || !messageSessionID || part.type !== "text") return; | ||
|
|
||
| rememberTextPart(part); | ||
| if (!assistantMessageSessionIds.has(part.messageID)) return; | ||
| if (isTextPartComplete(part) || completedAssistantMessageIds.has(part.messageID)) { | ||
| await captureAssistantMessage(part.messageID, messageSessionID, projectName, ctx.directory); | ||
| } | ||
| break; |
Contributor
There was a problem hiding this comment.
When message.updated (with finish) fires and captureAssistantMessage is called with no text yet (returns early, not marked captured), any subsequent message.part.updated events check completedAssistantMessageIds.has(part.messageID) and trigger capture on the first arriving part — which immediately sets capturedAssistantMessageIds.add(messageID). Any parts arriving after that first one are stored in textPartsByMessageId but skipped in captureAssistantMessage because the deduplication guard fires first, producing an incomplete observation with only the first fragment of the assistant message.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/integrations/opencode-plugin/index.ts
Line: 324-334
Comment:
**Partial capture when message completes before all parts arrive**
When `message.updated` (with `finish`) fires and `captureAssistantMessage` is called with no text yet (returns early, not marked captured), any subsequent `message.part.updated` events check `completedAssistantMessageIds.has(part.messageID)` and trigger capture on the **first** arriving part — which immediately sets `capturedAssistantMessageIds.add(messageID)`. Any parts arriving after that first one are stored in `textPartsByMessageId` but skipped in `captureAssistantMessage` because the deduplication guard fires first, producing an incomplete observation with only the first fragment of the assistant message.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+209
to
+218
| function pruneAssistantMessageCache(): void { | ||
| while (assistantMessageSessionIds.size > MAX_MESSAGE_CACHE_ENTRIES) { | ||
| const oldestMessageId = assistantMessageSessionIds.keys().next().value; | ||
| if (oldestMessageId === undefined) break; | ||
| assistantMessageSessionIds.delete(oldestMessageId); | ||
| completedAssistantMessageIds.delete(oldestMessageId); | ||
| capturedAssistantMessageIds.delete(oldestMessageId); | ||
| textPartsByMessageId.delete(oldestMessageId); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
textPartsByMessageId not bounded by pruneAssistantMessageCache
rememberTextPart writes to textPartsByMessageId for every message.part.updated event, regardless of whether the message ID is tracked in assistantMessageSessionIds. Because pruneAssistantMessageCache only evicts entries that exist in assistantMessageSessionIds, parts for messages that never appear in a message.updated event (e.g., user messages that only emit part events, or parts for abandoned streams) accumulate in textPartsByMessageId without bound and are never pruned.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/integrations/opencode-plugin/index.ts
Line: 209-218
Comment:
**`textPartsByMessageId` not bounded by `pruneAssistantMessageCache`**
`rememberTextPart` writes to `textPartsByMessageId` for every `message.part.updated` event, regardless of whether the message ID is tracked in `assistantMessageSessionIds`. Because `pruneAssistantMessageCache` only evicts entries that exist in `assistantMessageSessionIds`, parts for messages that never appear in a `message.updated` event (e.g., user messages that only emit part events, or parts for abandoned streams) accumulate in `textPartsByMessageId` without bound and are never pruned.
How can I resolve this? If you propose a fix, please make it concise.| // text capture path (see REAL_OPENCODE_EVENT_TYPES). | ||
| event: async ({ event }: { event: BusEvent }): Promise<void> => { | ||
| const eventType = event?.type as RealOpenCodeEventType | undefined; | ||
| const sessionID = event?.properties?.sessionID || event?.properties?.info?.id; |
Contributor
There was a problem hiding this comment.
info?.id is a message ID, not a session ID
The fallback event?.properties?.info?.id reads from OpenCodeMessageInfo.id, which is the message identifier. For session.idle and session.deleted events, this field would represent a message ID, not a session ID. If properties.sessionID were ever absent on those events, the wrong ID would be used as the session key — silently failing to clean up or summarize the correct session.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/integrations/opencode-plugin/index.ts
Line: 309
Comment:
**`info?.id` is a message ID, not a session ID**
The fallback `event?.properties?.info?.id` reads from `OpenCodeMessageInfo.id`, which is the message identifier. For `session.idle` and `session.deleted` events, this field would represent a message ID, not a session ID. If `properties.sessionID` were ever absent on those events, the wrong ID would be used as the session key — silently failing to clean up or summarize the correct session.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+123
to
+172
| it("posts assistant observations from message.updated and message.part.updated events", async () => { | ||
| const posts: Array<{ url: string; body: unknown }> = []; | ||
| const originalFetch = globalThis.fetch; | ||
| globalThis.fetch = (async (url: string | URL | Request, init?: RequestInit) => { | ||
| posts.push({ | ||
| url: String(url), | ||
| body: init?.body ? JSON.parse(String(init.body)) : null, | ||
| }); | ||
| return new Response(JSON.stringify({ status: "queued" }), { status: 200 }); | ||
| }) as typeof fetch; | ||
|
|
||
| try { | ||
| const plugin = await ClaudeMemPlugin(pluginCtx); | ||
| await plugin.event({ | ||
| event: { | ||
| type: "message.updated", | ||
| properties: { | ||
| sessionID: "ses_msg_1", | ||
| info: { id: "msg_1", role: "assistant", sessionID: "ses_msg_1" }, | ||
| }, | ||
| }, | ||
| }); | ||
| await plugin.event({ | ||
| event: { | ||
| type: "message.part.updated", | ||
| properties: { | ||
| sessionID: "ses_msg_1", | ||
| part: { | ||
| id: "prt_1", | ||
| sessionID: "ses_msg_1", | ||
| messageID: "msg_1", | ||
| type: "text", | ||
| text: "assistant text", | ||
| time: { end: 1 }, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| const initPost = posts.find((p) => p.url.includes("/api/sessions/init")); | ||
| const obsPost = posts.find((p) => p.url.includes("/api/sessions/observations")); | ||
| expect(initPost, "message events should lazily init the session").toBeTruthy(); | ||
| expect(obsPost, "message events should POST an assistant observation").toBeTruthy(); | ||
| const obsBody = obsPost!.body as Record<string, unknown>; | ||
| expect(obsBody.tool_name).toBe("assistant_message"); | ||
| expect(obsBody.tool_response).toBe("assistant text"); | ||
| } finally { | ||
| globalThis.fetch = originalFetch; | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
assistantMessageSessionIds, capturedAssistantMessageIds, textPartsByMessageId, and initializedSessionIds are module-level singletons shared across all test cases within the same process. If the test suite is run again (e.g., Bun watch mode or a second bun test invocation in the same process), ses_msg_1 will already be in initializedSessionIds (no init POST sent) and msg_1 will already be in capturedAssistantMessageIds (no observation POST sent), causing expect(initPost).toBeTruthy() and expect(obsPost).toBeTruthy() to fail.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/integrations/opencode-plugin-contract.test.ts
Line: 123-172
Comment:
**Module-level state causes flaky assertions on repeated test runs**
`assistantMessageSessionIds`, `capturedAssistantMessageIds`, `textPartsByMessageId`, and `initializedSessionIds` are module-level singletons shared across all test cases within the same process. If the test suite is run again (e.g., Bun watch mode or a second `bun test` invocation in the same process), `ses_msg_1` will already be in `initializedSessionIds` (no init POST sent) and `msg_1` will already be in `capturedAssistantMessageIds` (no observation POST sent), causing `expect(initPost).toBeTruthy()` and `expect(obsPost).toBeTruthy()` to fail.
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When I was using OpenCode to use claude-mem, the hooks doesn't initialize, therefore leading to empty knowledge bank of session summaries. Turns out there's a problem: the plugin was still using the stale chat.message hook, and its worker POSTs were fire-and-forget, so session init and observation requests could be dropped.
Summary
Replace stale chat.message OpenCode capture path with current message.updated / message.part.updated bus events
Await worker POSTs for session init, observations, and summaries so capture requests are not dropped
Update OpenCode contract tests to guard against stale hook/event regressions
Verification
bun test tests/integrations/opencode-plugin-contract.test.ts tests/integration/opencode-installer.test.ts
npm run typecheck:root
npm run build