Skip to content

Improve shutdown handling in WebSocket serve functions - #32

Merged
Leoyzen merged 4 commits into
wolf1069b:develop/agenticfrom
A-Qiang:develop/agentic
May 25, 2026
Merged

Improve shutdown handling in WebSocket serve functions#32
Leoyzen merged 4 commits into
wolf1069b:develop/agenticfrom
A-Qiang:develop/agentic

Conversation

@A-Qiang

@A-Qiang A-Qiang commented May 23, 2026

Copy link
Copy Markdown

No description provided.

@gemini-code-assist

Copy link
Copy Markdown

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@Leoyzen

Leoyzen commented May 25, 2026

Copy link
Copy Markdown
Collaborator

/gemini review

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

Copy link
Copy Markdown

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 refactors the elicitation system to support distinct 'form' and 'url' modes, updating the schema, request methods, and capability checks. It also simplifies transport connection handling by waiting directly on internal receive tasks and changes several capability fields to be optional by default. Feedback focuses on improving the robustness of the transport layer by removing '.done()' checks that could cause hangs and cautions against accessing private members of external libraries. Additionally, the reviewer recommends ensuring proper null guards are in place for the newly optional capability fields.

Comment thread src/acp/transports.py Outdated
Comment thread src/acp/transports.py Outdated
Comment thread src/acp/schema/agent_responses.py
Comment thread src/acp/transports.py Outdated
Comment thread src/acp/transports.py Outdated
…ing .done() checks and adding runtime guards
@A-Qiang

A-Qiang commented May 25, 2026

Copy link
Copy Markdown
Author

/gemini review

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

Copy link
Copy Markdown

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 refactors the elicitation process by introducing distinct 'form' and 'url' modes, updating the associated schemas and capability checks accordingly. It also simplifies capability defaults to null and adds a validator to handle empty objects for backward compatibility. Feedback was provided regarding the transport layer's reliance on private attributes (_conn and _recv_task), which is considered brittle and violates encapsulation.

Comment thread src/acp/transports.py Outdated
Comment thread src/acp/transports.py Outdated
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