1. src/agents/pi-embedded-subscribe.handlers.messages.ts, line 134-153 (link)

   thinking_end hook can fire multiple times

   The thinking_start hook is correctly guarded by if (!ctx.state.reasoningStreamOpen) so it fires exactly once. The thinking_end hook has no equivalent guard.

   When thinking_end arrives, lines 135–136 forcefully set reasoningStreamOpen = true (to handle providers that skip thinking_start/thinking_delta), then emitReasoningEnd resets it to false. If a second thinking_end event arrives (which some providers do send), the same path repeats — reasoningStreamOpen is forced back to true, emitReasoningEnd fires again, and the hook fires a second time.

   Plugin authors subscribing to thinking_end will be surprised to receive it more than once per thinking block, especially since thinking_start is guaranteed to fire only once. Consider adding a guard analogous to the one used for thinking_start, or track a separate boolean (e.g., thinkingEndHookFired) that is reset at the same boundary as reasoningStreamOpen:

   if (evtType === "thinking_end") {
     if (!ctx.state.reasoningStreamOpen) {
       ctx.state.reasoningStreamOpen = true;
     }
     emitReasoningEnd(ctx);
     // Emit thinking_end hook — only if the stream was actually open (i.e. we had a thinking phase)
     if (!ctx.state.reasoningEndHookFired) {
       ctx.state.reasoningEndHookFired = true;
       const hookRunner = ctx.hookRunner ?? getGlobalHookRunner();
       if (hookRunner?.hasHooks("thinking_end")) {
         // ...
       }
     }
   }

   (Reset reasoningEndHookFired at the same place reasoningStreamOpen is reset — emitReasoningEnd or resetAssistantMessageState.)

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/agents/pi-embedded-subscribe.handlers.messages.ts
   Line: 134-153
   
   Comment:
   **`thinking_end` hook can fire multiple times**
   
   The `thinking_start` hook is correctly guarded by `if (!ctx.state.reasoningStreamOpen)` so it fires exactly once. The `thinking_end` hook has no equivalent guard.
   
   When `thinking_end` arrives, lines 135–136 _forcefully_ set `reasoningStreamOpen = true` (to handle providers that skip `thinking_start`/`thinking_delta`), then `emitReasoningEnd` resets it to `false`. If a second `thinking_end` event arrives (which some providers do send), the same path repeats — `reasoningStreamOpen` is forced back to `true`, `emitReasoningEnd` fires again, and the hook fires a second time.
   
   Plugin authors subscribing to `thinking_end` will be surprised to receive it more than once per thinking block, especially since `thinking_start` is guaranteed to fire only once. Consider adding a guard analogous to the one used for `thinking_start`, or track a separate boolean (e.g., `thinkingEndHookFired`) that is reset at the same boundary as `reasoningStreamOpen`:
   
   ```typescript
   if (evtType === "thinking_end") {
     if (!ctx.state.reasoningStreamOpen) {
       ctx.state.reasoningStreamOpen = true;
     }
     emitReasoningEnd(ctx);
     // Emit thinking_end hook — only if the stream was actually open (i.e. we had a thinking phase)
     if (!ctx.state.reasoningEndHookFired) {
       ctx.state.reasoningEndHookFired = true;
       const hookRunner = ctx.hookRunner ?? getGlobalHookRunner();
       if (hookRunner?.hasHooks("thinking_end")) {
         // ...
       }
     }
   }
   ```
   
   (Reset `reasoningEndHookFired` at the same place `reasoningStreamOpen` is reset — `emitReasoningEnd` or `resetAssistantMessageState`.)
   
   How can I resolve this? If you propose a fix, please make it concise.

Best possible solution:

Rebase onto current main, use the host-owned grouped plugin event subscription contract unless maintainers approve a new top-level API, make thinking lifecycle delivery deterministic, and provide redacted real plugin/channel proof before merge.

Do we have a high-confidence way to reproduce the issue?

Yes for the review findings: source inspection of current main and PR head reproduces the API-shape mismatch, duplicate native thinking_end path, and regex-only fallback extraction. I did not run tests because this review is read-only.

Is this the best way to solve the issue?

No, not as-is. The direction is plausible, but the maintainable solution should align with current main's grouped plugin event contract and guarantee one-to-one thinking lifecycle edges before exposing the SDK surface.

Full review comments:

• [P2] Wire the event API through current builders — src/plugins/types.ts:1392-1395
  Adding a required top-level OpenClawPluginApi.onAgentEvent member conflicts with current main's api.agent.events.registerAgentEventSubscription(...) facade and API builder. Rebase this onto the grouped event contract or update every current builder/helper that constructs OpenClawPluginApi, or SDK/helper consumers will see a required member that the host does not consistently provide.
  Confidence: 0.86
• [P2] Ignore duplicate native thinking_end events — src/agents/pi-embedded-subscribe.handlers.messages.ts:223-241
  After a normal native thinking block closes, a repeated provider thinking_end can enter this synthetic-start path and emit another thinking lifecycle pair for the same reasoning block. Track the closed native block or ignore duplicate terminal events so plugin UIs do not double-count or flicker.
  Confidence: 0.86
• [P2] Use code-span-aware fallback extraction — src/agents/pi-embedded-subscribe.handlers.messages.ts:463-465
  The message_end fallback extracts tagged thinking with a regex-only helper. If native thinking is open and the final answer contains a literal <think>...</think> example in code, plugins can receive fake reasoning text; reuse the code-span-aware parser or avoid parsing user-visible code in this fallback.
  Confidence: 0.78

Overall correctness: patch is incorrect
Overall confidence: 0.86

What I checked:

• Live PR state: The PR is open, external, non-draft, authored by Carlos Gustavo Sarmiento (@carlos-sarmiento, account created 2012-06-28); latest head is 22b6034 and CI still has two failing pnpm test:channels shards. (22b603402ba1)
• Current main plugin event contract: Current main exposes plugin event subscriptions as api.agent.events.registerAgentEventSubscription(...) and emitAgentEvent(...), not a new top-level api.onAgentEvent(...) API. (src/plugins/types.ts:2502, 936989a88b1d)
• Current main API builder wiring: buildPluginApi(...) wires registerAgentEventSubscription and emitAgentEvent, then attaches grouped facades; it has no top-level onAgentEvent member. (src/plugins/api-builder.ts:256, 936989a88b1d)
• Current docs prefer grouped SDK namespaces: The plugin SDK overview documents api.agent.events.registerAgentEventSubscription(...) as the grouped namespace for sanitized event subscriptions and says new plugin code should use grouped namespaces. Public docs: docs/plugins/sdk-overview.md. (docs/plugins/sdk-overview.md:152, 936989a88b1d)
• Current main partially overlaps but does not cover the PR: Current main already emits stream: "thinking" agent events from emitReasoningStream, but only when state.streamReasoning and params.onReasoningStream are present, so it does not fully replace the PR's plugin-facing event API and hook additions. (src/agents/pi-embedded-subscribe.ts:878, 936989a88b1d)
• Current main lacks thinking hook names: The current PluginHookName union does not include thinking_start or thinking_end, so the PR still has unique SDK surface beyond main. (src/plugins/hook-types.ts:68, 936989a88b1d)

Likely related people:

• 100yenadmin: Authored the generic plugin host-hook contracts commit that introduced the current registerAgentEventSubscription path this PR must align with. (role: introduced behavior; confidence: high; commits: 1adaa28dc86b; files: src/plugins/types.ts, src/plugins/api-builder.ts, src/plugins/registry.ts)
• jalehman: Listed as reviewer and co-author on the generic plugin host-hook contracts commit that owns the current plugin event subscription contract. (role: reviewer; confidence: medium; commits: 1adaa28dc86b; files: src/plugins/types.ts, src/plugins/host-hook-runtime.ts, src/plugins/registry.ts)
• medns: Recently refactored raw reasoning streaming and touched the subscribe/message utility path central to thinking-event behavior. (role: recent area contributor; confidence: medium; commits: e7277b4e3a4b; files: src/agents/pi-embedded-subscribe.handlers.messages.ts, src/agents/pi-embedded-subscribe.ts, src/agents/pi-embedded-utils.ts)
• cathrynlavery: Recently changed src/infra/agent-events.ts session-key preservation for hidden lifecycle events, directly adjacent to the session-scoping concern for plugin event subscriptions. (role: recent area contributor; confidence: medium; commits: 763a88083e63; files: src/infra/agent-events.ts, src/infra/agent-events.test.ts)

Remaining risk / open question:

• The external PR lacks after-fix real behavior proof from an actual plugin/channel setup.
• The proposed public SDK shape needs maintainer API judgment because current main now documents grouped api.agent.events.* namespaces.
• Latest PR-head CI still has failing channel test shards, and this read-only review did not rerun tests locally.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 936989a88b1d.