Skip to content

fix(server): cast finished listener call, remove stale TODO, document execute contract - #553

Merged
JakubWorek merged 2 commits into
epic/1.0_breaking_changesfrom
jakubworek/fix-sdk-small-cleanups
Jun 29, 2026
Merged

fix(server): cast finished listener call, remove stale TODO, document execute contract#553
JakubWorek merged 2 commits into
epic/1.0_breaking_changesfrom
jakubworek/fix-sdk-small-cleanups

Conversation

@JakubWorek

Copy link
Copy Markdown
Contributor

Description

Three small SDK quality fixes surfaced while reviewing the codebase:

SDK changes

  • execution_event_bus: Cast listener.call(this) for 'finished' listeners (which are invoked with no args despite the typed Listener signature). Clears 2 TS strict-mode issues; .betterer.results regression baseline shrinks accordingly.
  • jsonrpc_transport_handler: Removed stale TODO about mid-stream SSE errors; the Express layer already catches and writes formatSSEErrorEvent (covered by express_app.spec.ts).
  • AgentExecutor.execute JSDoc: Documented the contract that every streaming turn must begin with a task or message event (including follow-ups). The server already enforces this; the JSDoc just makes it explicit to implementors.

@JakubWorek
JakubWorek requested a review from a team as a code owner June 29, 2026 10:00
@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown

🧪 Code Coverage

⬇️ Download Full Report

Base PR Delta
src/compat/v0_3/server/transports/jsonrpc/jsonrpc_transport_handler.ts 97.11% 100% 🟢 +2.89%
src/server/transports/jsonrpc/jsonrpc_transport_handler.ts 78.89% 80.95% 🟢 +2.06%
Total 90.73% 90.84% 🟢 +0.11%

Generated by coverage-comment.yml

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses TypeScript strict-mode compiler errors in execution_event_bus.ts by adding type casts, updates documentation in agent_executor.ts regarding event publishing requirements, and refactors comments in jsonrpc_transport_handler.ts. The reviewer provided valuable feedback, suggesting method overloading in ExecutionEventBus to achieve proper type safety without casting, and recommending the removal of a redundant console.error call in the JSON-RPC transport handler to prevent duplicate server logs.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/server/events/execution_event_bus.ts Outdated
Comment thread src/server/transports/jsonrpc/jsonrpc_transport_handler.ts Outdated
@JakubWorek
JakubWorek force-pushed the jakubworek/fix-sdk-small-cleanups branch from e2f15af to be71a19 Compare June 29, 2026 11:37
@JakubWorek
JakubWorek merged commit 775afad into epic/1.0_breaking_changes Jun 29, 2026
9 checks passed
@JakubWorek
JakubWorek deleted the jakubworek/fix-sdk-small-cleanups branch June 29, 2026 11:43
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.

2 participants