Skip to content

fix(openapi): query worker thread for REST OpenAPI spec on main thread - #652

Merged
kriszyp merged 3 commits into
mainfrom
kris/vigorous-black-9fca5c
May 21, 2026
Merged

fix(openapi): query worker thread for REST OpenAPI spec on main thread#652
kriszyp merged 3 commits into
mainfrom
kris/vigorous-black-9fca5c

Conversation

@kriszyp

@kriszyp kriszyp commented May 20, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds two new ITC event types (RESOURCE_OPENAPI_REQUEST / RESOURCE_OPENAPI_RESPONSE) following the existing COMPONENT_STATUS_REQUEST pattern
  • Worker handler in serverHandlers.js generates the OpenAPI spec from its local resources and returns it directly to the requesting thread; skips if resources are empty (guards against job-type workers responding before app workers)
  • restOpenAPIHandler in operationsServer.ts falls back to cross-thread query when resources.size === 0 (i.e. running on main thread with worker threads); uses local resources directly in single-thread mode
  • 5-second timeout with 503 error if no worker responds

Purpose

Resolves the concern from #299 introduced by moving the operations API to the main thread (PR #355): application resources are only registered on worker threads, so /api/openapi/rest was returning an empty spec when called on the main thread.

What to review

  • server/operationsServer.ts: queryWorkerForOpenApi / attachOpenApiResponseListener — the main-thread side of the query. The requestId counter and pending-request map follow the same pattern as CrossThreadStatusCollector in components/status/crossThread.ts.
  • server/itc/serverHandlers.js: resourceOpenApiRequestHandler — runs on worker threads; guards with resources.size === 0 so workers with no registered resources (e.g. job workers at startup) don't respond.
  • Design trade-off: All workers with resources will respond (first one wins, rest are silently discarded). An alternative would be to target only one worker, but that requires maintaining worker ID state and handling worker exit races — not worth the complexity for a low-frequency endpoint.

Test plan

  • Unit tests added in unitTests/server/itc/serverHandlers.test.js covering validation, happy-path response, and unreachable-originator drop — all passing
  • All 413 server unit tests pass
  • Manual verification: start Harper with multiple workers, call GET /api/openapi/rest as a super_user, confirm non-empty spec is returned

Generated by Claude Sonnet 4.6 (agent)

When the operations API runs on the main thread (post PR #355), resources
are only registered on worker threads, so /api/openapi/rest returned an
empty spec. This adds a cross-thread ITC mechanism: the main thread
broadcasts RESOURCE_OPENAPI_REQUEST; the first worker with registered
resources generates the spec via generateJsonApi and sends it back via
RESOURCE_OPENAPI_RESPONSE. Falls back to local resources when running in
single-thread mode (resources.size > 0).

Resolves #299 (partial — addresses the OpenAPI endpoint concern)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@kriszyp
kriszyp marked this pull request as ready for review May 20, 2026 22:51
@kriszyp
kriszyp requested a review from a team as a code owner May 20, 2026 22:51
Comment thread server/itc/serverHandlers.js Outdated
…OpenAPI requests

Without this change, a worker with 0 registered resources would silently drop
the ITC request, causing the main thread to wait 5 seconds before timing out.
Now every worker responds (even with an empty spec), matching Dawson's review request.

Also adds a full test suite for resourceOpenApiRequestHandler including
happy-path send/drop assertions and a 'no hang on empty resources' case.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread server/itc/serverHandlers.js
@claude

claude Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

…ly reply

Without the guard a job-type worker (resources.size === 0) responds to
RESOURCE_OPENAPI_REQUEST before an app worker, silently returning an empty
spec — identical to the original bug (#299).

Restores the early-return guard so only a worker that actually owns resources
replies. When no worker has resources the caller receives a 503 after the
timeout, which is more honest than an empty spec.

Also fixes the unit tests to properly exercise the guard and the happy-path:
- 'does not respond when this thread has no registered resources' verifies the guard
- Send/drop tests populate the resources map with a minimal skippable entry
  (isError: true) so Resources.set() wraps it correctly for generateJsonApi

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@kriszyp
kriszyp merged commit ad3fe4f into main May 21, 2026
39 checks passed
@kriszyp
kriszyp deleted the kris/vigorous-black-9fca5c branch May 21, 2026 20:41
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