Add typed agent-server compatibility helpers#176
Conversation
all-hands-bot
left a comment
There was a problem hiding this comment.
🟢 Good taste - Clean implementation with proper abstractions.
This PR adds type-safe agent-server version compatibility checking with intelligent caching. The semantic version parsing is solid, error handling is clear, and the WorkspacesClient integration is elegant. All changes are browser-compatible and backward compatible.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
This is a well-contained feature addition with comprehensive test coverage. The version checking logic prevents runtime errors by failing fast with clear error messages. No breaking changes, no dependencies added, and the caching strategy using WeakMap ensures proper garbage collection.
VERDICT:
✅ Worth merging - Solid implementation, no issues found.
KEY INSIGHT:
The cached version check prevents redundant server_info calls while still allowing per-HttpClient-instance version verification, striking a good balance between performance and correctness.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/typescript-client/actions/runs/26331156132
all-hands-bot
left a comment
There was a problem hiding this comment.
🟢 Good taste - Clean implementation with proper abstractions and elegant version checking.
The semantic version parsing is solid, caching strategy is smart (WeakMap), and the error handling provides clear feedback. All changes are browser-compatible and backward compatible. Just one minor type safety improvement suggested.
[IMPROVEMENT OPPORTUNITIES]
- [src/client/conversation-client.ts, Line 141] Type Safety: Use the existing
LLMinterface instead ofunknownfor better type safety and IDE support.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
This is a well-contained additive feature with comprehensive test coverage. Version checking logic is correct, all changes are backward compatible, and the caching mechanism is elegant. No breaking changes or security concerns.
VERDICT:
✅ Worth merging: Clean architecture with one minor type improvement suggested
KEY INSIGHT:
The WeakMap-based caching elegantly solves the "check once, trust always" problem while automatically cleaning up when client instances are garbage collected.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/typescript-client/actions/runs/26331261532
Summary
AgentServerVersionError1.23.0WorkspacesClientso workspace endpoints fail with a typed version error before hitting missing routes on old serversRemoteEventsListconstruction from client options and addConversationClient.switchLLMso canvas callers do not need to instantiateHttpClientdirectlyVerification
npm run buildnpm run format:checknpm run lint(existing warnings only)npm test -- --runTestsByPath src/__tests__/api-clients.test.tsnpm test