feat(client): add InitClient for the deferred-init endpoints - #243
feat(client): add InitClient for the deferred-init endpoints#243VascoSch92 wants to merge 2 commits into
Conversation
Adds coverage for the warm-pool deferred-init routes the client was missing: - GET /api/init -> InitClient.getStatus() - POST /api/init -> InitClient.initialize() (sends the bootstrap X-Init-API-Key header, distinct from the per-session key) New src/client/init-client.ts, InitState/InitStatus/InitRequest model types, exports from clients.ts and index.ts, and an `init` namespace on ConversationManager. Includes unit tests (namespace wiring, getStatus, initialize with and without the init key) and an integration contract guard (both routes 404 on the non-deferred pinned image, proving the paths/methods exist).
Endpoint audit❌ 7 off-contract call(s) — not on the agent-server · classifiers: cloud
❌ Not on agent-server (gated, 7)⛔ (no known backend) — served by no backend we can see (6)
|
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
There was a problem hiding this comment.
Reviewed this end-to-end. The implementation is small, idiomatic, and follows the conventions established by BashClient, HooksClient, etc. — HttpClient wrapper, host.replace(/\/$/, '') normalization, close() parity, model types colocated in models/api.ts, exposed via clients.ts and index.ts. The deferred-init integration test is genuinely useful: asserting 404 (not 405/422) proves both that the routes exist on the pinned image and that the client targets the right path/method — that's a real contract test, not a mock-pile. CI is green across all 11 checks. Two minor doc clarifications below; otherwise ready to merge.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
|
@OpenHands address the comments and fix the conflicts. |
|
I'm on it! VascoSch92 can track my progress at all-hands.dev |
Resolve conflicts with main and clarify InitClient header behavior. Co-authored-by: openhands <[email protected]>
|
Done — PR #243 is updated: #243 Summary:
Validation:
Current PR state: conflicts are fixed; merge is still blocked only by required review ( |
|
🤖 OpenHands is reviewing this PR. Head commit: This comment was posted by an AI agent (OpenHands). |
all-hands-bot
left a comment
There was a problem hiding this comment.
This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.
Review: feat(client): add InitClient for the deferred-init endpoints
Verdict: No material findings — ready to merge.
Risk assessment: LOW.
The change adds InitClient covering GET /api/init and POST /api/init for the warm-pool deferred-init lifecycle. The implementation is small, idiomatic, and consistent with the existing client conventions:
HttpClientwrapper withhost.replace(/\/$/, '')normalization andclose()parity.initialize()correctly sendsX-Init-API-Keyonly when supplied and omits it otherwise (verified by a dedicated unit test). The header/body semantics match the agent-server init router.- Model types (
InitState/InitStatus/InitRequest) are colocated inmodels/api.tsand exported from bothclients.ts(class + options types) andindex.ts(model types only) — consistent with how the other secondary clients are surfaced. - Wired into
ConversationManageras theinitnamespace, constructed and closed alongside the other clients. - The deterministic integration test asserting
404(not405/422) on the non-deferred pinned image is a genuine contract guard — it proves both routes exist on the image and that the client targets the correct path/method, which mocked unit tests cannot catch.
Points verified, no action needed
- Dual-header behavior on
POST /api/init: whenConversationManageris constructed with anapiKey,HttpClientattachesX-Session-API-Keyon every request including the init calls. This is harmless — the init router authenticates viaX-Init-API-Keyand ignores the session header — and the doc comment ongetStatus/initializealready guides callers to omitapiKeyfor dormant polling. Not a bug. secret_keyinInitRequest: transmitted in the POST body over HTTPS and authenticated byX-Init-API-Key; matches the server's intended design.- Default-body
initialize():request = {}producesJSON.stringify({}), verified by the unit test. - Export surface:
InitClientis exported from the./clientssubpath (likeServerClient,BashClient, etc.); the model types are additionally exported from the root entry point. Consistent with the existing pattern.
No bugs, security problems, or design flaws identified.
Summary
Split from #231 (one feature per PR). Adds client coverage for the warm-pool deferred-init endpoints that the audit reported as missing. Implementations follow the agent-server init router in
OpenHands/software-agent-sdk(see software-agent-sdk#2523).GET /api/initInitClient.getStatus()POST /api/initInitClient.initialize()(sendsX-Init-API-Key)Details
src/client/init-client.tswraps the deferred-init / warm-pool endpoints.POST /api/initauthenticates with the bootstrapX-Init-API-Keyheader (distinct from the per-sessionX-Session-API-Key); it is omitted when no key is given.InitState/InitStatus/InitRequestadded tomodels/api.tsand exported fromindex.ts.InitClientexported fromclients.tsand wired intoConversationManageras theinitnamespace (constructed and closed alongside the other clients).Testing
src/__tests__/api-clients.test.ts, mockedfetch):initnamespace wiring,getStatusGET,initializePOST (header + body present), andinitializeomitting the header when no key is given.deterministic-api.integration.test.ts): both routes return404on the non-deferred pinned image (get_init_serviceanswers 404 when noInitServiceis registered) — proving the GET/POST routes exist on the image and the client targets the right path/method.npm run build,lint(0 errors), andformat:checkpass; full unit suite green (273).