Conversation
…th JSON-RPC support
…lient from the model
…AcpServer base structure this is just a first pass, significant refactoring is pending
…to simplify server handleTurnUpdate logic
…specified during tool construction
oca-agent
left a comment
There was a problem hiding this comment.
Code Review Summary
Overall, this is a solid implementation. Refactoring Agent -> Session clarifies the codebase architecture (distinguishing the overall agent application/service from an individual context window session), and the new acp module is clean, well-structured, and thoroughly tested. The compile-time enum string mapping (session_new <-> session/new) is particularly slick with zero runtime allocation overhead.
Below are inline comments highlighting a panic hazard, error handling edge cases, and protocol robustness suggestions to help guide the implementation before taking this out of draft.
…que pointer in the tool object and prevent leaking into tool function definitions
…the callers POV and manage pointer casting internally
oca-agent
left a comment
There was a problem hiding this comment.
Code Review Summary (Round 2)
Excellent progress! The recent updates address almost all feedback from the initial pass:
- ✅ Server Error Recovery:
Server.run()now catches request errors and returns proper JSON-RPC error responses rather than terminating the loop. - ✅ Type-Safe Context Passing:
Tool.initWithContextandToolCallContextnow use compile-time type matching for context pointers (isContextTypeCompatible) without forcing?*anyopaqueunwrapping (ctx.?). - ✅ Case-Insensitive Headers:
JsonRpcReadernow usesstd.ascii.startsWithIgnoreCaseforContent-Length:header checks. - ✅ Performance & Struct Cleanup: Moved
SessionStoragetoStringHashMapUnmanagedand removed unused context pointers. - ✅ ACP Streaming Tool Call Updates: Added support for streaming tool call updates (
tool_call_update) within_progressandcompletedstatuses. - ✅ Built-In Todo Tool: Cleanly implemented with full unit test coverage.
Below are inline comments highlighting two remaining items (a panic hazard on empty prompts and an issue with injected context step placement during multi-step tool turns).
…rt prompt array inputs in session/prompt server method
oca-agent
left a comment
There was a problem hiding this comment.
All comments and edge cases have been resolved. The PR is well-tested (208/208 tests passing), well-structured, and ready to merge! LGTM!
Summary
This PR implements the initial Agent Communication Protocol (ACP) server specification over JSON-RPC 2.0. It enables client applications to communicate with
comaas an agent service over stdio/JSON-RPC, managing agent sessions, executing turns, and streaming thought and message content back to clients in real-time.Additionally, this PR refactors core agent types to align with ACP session semantics (
Agent→Session) and introduces support for context-aware tool execution.