Skip to content

Add typed agent-server compatibility helpers#176

Merged
neubig merged 3 commits into
mainfrom
codex/agent-server-version-errors
May 23, 2026
Merged

Add typed agent-server compatibility helpers#176
neubig merged 3 commits into
mainfrom
codex/agent-server-version-errors

Conversation

@neubig
Copy link
Copy Markdown
Member

@neubig neubig commented May 23, 2026

Summary

  • add an agent-server compatibility helper with AgentServerVersionError
  • define the workspace minimum server version as 1.23.0
  • add and export WorkspacesClient so workspace endpoints fail with a typed version error before hitting missing routes on old servers
  • allow RemoteEventsList construction from client options and add ConversationClient.switchLLM so canvas callers do not need to instantiate HttpClient directly

Verification

  • npm run build
  • npm run format:check
  • npm run lint (existing warnings only)
  • npm test -- --runTestsByPath src/__tests__/api-clients.test.ts
  • npm test

@neubig neubig changed the title Add typed agent-server version errors for workspaces Add typed agent-server compatibility helpers May 23, 2026
Copy link
Copy Markdown

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

🟢 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

@neubig neubig marked this pull request as ready for review May 23, 2026 11:17
Copy link
Copy Markdown

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

🟢 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 LLM interface instead of unknown for 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

Comment thread src/client/conversation-client.ts Outdated
@neubig neubig merged commit 33d245a into main May 23, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants