Skip to content

fix(plugins): deliver onStreamComplete to disk-installed plugins (#11825) - #11934

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
pacocartones:fix/plugin-onstreamcomplete-delivery
Aug 30, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
pacocartones:fix/plugin-onstreamcomplete-delivery

Conversation

@pacocartones

Copy link
Copy Markdown
Contributor

Summary

  • onStreamComplete (shipped in v3.8.50, feat(plugins): add onStreamComplete built-in event exposing streaming usage and timing (#9571) #9669) was emitted on the gateway side but never delivered to disk-installed plugins — the plugin-facing wiring was missing, so no plugin could subscribe to it.
  • Root cause, three layers: the manifest HooksSchema had no onStreamComplete field (Zod silently stripped it from plugin.json); loader.ts built no IPC wrapper for it; and manager.ts's hardcoded hookNames list never registered it. The event fired into an empty registry and was dropped. The existing chatcore-plugin-onresponse tests only passed because they call registerHook("onStreamComplete", …) directly — a path real plugins don't have.
  • Fix: declare onStreamComplete as a manifest hook, wire it through the loader as a fire-and-forget notification (same pattern as the lifecycle hooks), and register it in the manager. The payload now also carries requestId (the request traceId) so consumers can correlate the stream-completion event with onRequest/onResponse for the same request.

Related Issues

Validation

  • Change type: plugins (manifest/loader/manager) + streaming finalization wiring
  • Focused test: tests/unit/plugins-onstreamcomplete-delivery-11825.test.ts — drives the real install → activate → emit path (not registerHook directly). Verified failing on the pre-fix tree (manifest.hooks.onStreamComplete must survive validation; installed plugin should declare onStreamComplete; got "[]") and passing after (2/2).
  • npm run lint (pre-commit prettier + eslint ran clean on all touched files)
  • Reconciled with the current main; focused test rerun afterward
  • Production-code changes include a new automated test in this PR

Tests Added Or Updated

  • Added tests/unit/plugins-onstreamcomplete-delivery-11825.test.ts: installs + activates a plugin declaring onStreamComplete, emits through runPluginOnStreamCompleteHook, and asserts the plugin process receives the payload including requestId. Also asserts the manifest schema preserves the hook flag.

Reviewer Notes

  • onStreamComplete is wired alongside the lifecycle hooks because it is a one-way fire-and-forget notification (no return value consumed); no blocking-hook path changes.
  • No change to the unrelated internal onStreamComplete stream-finalization callback in streamFailureFinalization.ts / streamingCost.ts — that is a different concept.

@pacocartones
pacocartones force-pushed the fix/plugin-onstreamcomplete-delivery branch from af5f241 to 9387e49 Compare August 28, 2026 19:19
…gosouzapw#11825)

onStreamComplete (shipped in v3.8.50, diegosouzapw#9669) was emitted on the gateway side
but never delivered to disk-installed plugins: the manifest HooksSchema dropped
hooks.onStreamComplete, and the loader/manager only knew the seven legacy hooks.
Declare it as a manifest hook, wire it through the loader as a fire-and-forget
notification, register it in the manager, and thread requestId (the request
traceId) into the payload so consumers can correlate the event.
@pacocartones
pacocartones force-pushed the fix/plugin-onstreamcomplete-delivery branch from 9387e49 to af7675d Compare August 29, 2026 02:49
@pacocartones

Copy link
Copy Markdown
Contributor Author

CI note — the red checks are not this change's doing (delivering onStreamComplete to disk-installed plugins). Build was cancelled by fail-fast, not a compile error. Vitest (MCP / autoCombo / UI components) and Protocol Clients E2E (advisory) fail environmentally (green on main; the run logs ECONNREFUSED and a better-sqlite3 binding fallback), on tests unrelated to plugin delivery. Rebased onto current main.

@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.51 August 30, 2026 08:16
…n-only antigravity lock-exact-model drift)
@diegosouzapw
diegosouzapw merged commit 66e02ec into diegosouzapw:release/v3.8.51 Aug 30, 2026
4 of 7 checks passed
diegosouzapw pushed a commit that referenced this pull request Aug 30, 2026
…ok fields reach existing installs (#12120)

Refresca o manifest do plugin a partir do disco ao ativar, para que instalações pré-existentes ganhem hooks novos adicionados por schema updates (ex.: `onStreamComplete` do #11825/#11934 nunca chegava a plugins já instalados antes do upgrade, pois o manifest persistido no DB era stripado pelo schema antigo). Finding 3 do #12113. Teste próprio (259 linhas). Validado no worktree combinado. Obrigado!
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…gosouzapw#11825) (diegosouzapw#11934)

Resynced onto release/v3.8.51 (originally targeted main; retargeted since the default branch is release/v3.8.51). One real conflict in open-sse/handlers/chatCore.ts, but it was entirely unrelated to this PR's actual purpose: the antigravity-aware lockExactModel branching and deferAntigravityQuotaStateToCaller state exist on main but haven't been synced to release/v3.8.51 yet (confirmed by diffing your branch against its own main merge-base — the only change there was a Prettier reformat, not new logic). Discarded that unrelated drift and kept the release tip's current quota-lock shape; the onStreamComplete plugin wiring itself is untouched and intact. typecheck:core clean, 13/13 plugin delivery tests pass. Thanks for the thorough three-layer root-cause writeup.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…ok fields reach existing installs (diegosouzapw#12120)

Refresca o manifest do plugin a partir do disco ao ativar, para que instalações pré-existentes ganhem hooks novos adicionados por schema updates (ex.: `onStreamComplete` do diegosouzapw#11825/diegosouzapw#11934 nunca chegava a plugins já instalados antes do upgrade, pois o manifest persistido no DB era stripado pelo schema antigo). Finding 3 do diegosouzapw#12113. Teste próprio (259 linhas). Validado no worktree combinado. Obrigado!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] onStreamComplete is emitted internally but never delivered to plugins (missing wiring in manifest schema / loader / manager)

2 participants