diff --git a/api/controllers/openapi/__init__.py b/api/controllers/openapi/__init__.py index f89ef0111c..4bd436418f 100644 --- a/api/controllers/openapi/__init__.py +++ b/api/controllers/openapi/__init__.py @@ -37,6 +37,8 @@ from controllers.openapi._models import ( DeviceMutateRequest, DeviceMutateResponse, DevicePollRequest, + FormSubmitResponse, + HealthResponse, MemberActionResponse, MemberInvitePayload, MemberInviteResponse, @@ -49,9 +51,11 @@ from controllers.openapi._models import ( PermittedExternalAppsListResponse, RevokeResponse, ServerVersionResponse, + SessionListQuery, SessionListResponse, SessionRow, TagItem, + TaskStopResponse, UsageInfo, WorkflowRunData, WorkspaceDetailResponse, @@ -74,6 +78,7 @@ register_schema_models( MemberListQuery, MemberRoleUpdatePayload, PermittedExternalAppsListQuery, + SessionListQuery, ) register_response_schema_models( openapi_ns, @@ -100,11 +105,14 @@ register_response_schema_models( MemberListResponse, MemberInviteResponse, MemberActionResponse, + TaskStopResponse, + FormSubmitResponse, DeviceCodeResponse, DeviceLookupResponse, DeviceMutateResponse, FileResponse, ServerVersionResponse, + HealthResponse, ) from . import ( diff --git a/api/controllers/openapi/_models.py b/api/controllers/openapi/_models.py index 59b2e5176e..d661f5640f 100644 --- a/api/controllers/openapi/_models.py +++ b/api/controllers/openapi/_models.py @@ -87,8 +87,12 @@ class AppDescribeInfo(AppInfoResponse): class AppDescribeResponse(BaseModel): info: AppDescribeInfo | None = None - parameters: dict[str, Any] | None = None - input_schema: dict[str, Any] | None = None + # `parameters` (the app-config blob) and `input_schema` (a Draft 2020-12 JSON Schema derived + # per-app) are deliberately open JSON, not under-annotated. The `x-dify-opaque` marker tells the + # contract generator's readiness detector to treat them as intentional, so the route is not + # flagged "annotations incomplete". CLI/web consume them as opaque objects either way. + parameters: dict[str, Any] | None = Field(default=None, json_schema_extra={"x-dify-opaque": True}) + input_schema: dict[str, Any] | None = Field(default=None, json_schema_extra={"x-dify-opaque": True}) class ChatMessageResponse(BaseModel): @@ -173,6 +177,15 @@ class SessionListResponse(BaseModel): data: list[SessionRow] +class SessionListQuery(BaseModel): + """Pagination for GET /account/sessions. Strict (extra='forbid').""" + + model_config = ConfigDict(extra="forbid") + + page: int = Field(1, ge=1) + limit: int = Field(100, ge=1, le=MAX_PAGE_LIMIT) + + class RevokeResponse(BaseModel): status: str @@ -223,6 +236,23 @@ class ServerVersionResponse(BaseModel): edition: Literal["SELF_HOSTED", "CLOUD"] +class HealthResponse(BaseModel): + """Liveness payload for `GET /openapi/v1/_health` — no auth required.""" + + ok: bool + + +def _csv_string_query_schema(schema: dict[str, Any]) -> None: + """Re-shape a set/list field's query schema to a comma-separated string — the wire form the + handler actually accepts (`request.args` is flat + the validator splits on ','). Without this + the generated contract would type it as an array and serialize `fields[0]=…&fields[1]=…`, + which `extra='forbid'` rejects. Runtime `set[str]` validation is unaffected.""" + schema.pop("anyOf", None) + schema.pop("items", None) + schema.pop("uniqueItems", None) + schema["type"] = "string" + + class AppDescribeQuery(BaseModel): """`?fields=` allow-list for GET /apps//describe. @@ -231,7 +261,7 @@ class AppDescribeQuery(BaseModel): model_config = ConfigDict(extra="forbid") - fields: set[str] | None = None + fields: set[str] | None = Field(default=None, json_schema_extra=_csv_string_query_schema) workspace_id: str | None = None @field_validator("workspace_id", mode="before") @@ -400,3 +430,19 @@ class MemberInviteResponse(BaseModel): class MemberActionResponse(BaseModel): result: Literal["success"] = "success" + + +class TaskStopResponse(BaseModel): + """200 body for POST /apps//tasks//stop. The handler always returns + {"result": "success"}, so `result` is required (no default) — the generated contract + types it as a required `'success'` rather than an optional field.""" + + result: Literal["success"] + + +class FormSubmitResponse(BaseModel): + """Empty 200 body for POST /apps//form/human_input/. `extra='forbid'` + pins `additionalProperties: false` so the generated contract is an exact `{}` rather + than an under-annotated open object.""" + + model_config = ConfigDict(extra="forbid") diff --git a/api/controllers/openapi/account.py b/api/controllers/openapi/account.py index 256a822dcb..05223a97e6 100644 --- a/api/controllers/openapi/account.py +++ b/api/controllers/openapi/account.py @@ -4,15 +4,17 @@ from datetime import UTC, datetime from flask import request from flask_restx import Resource -from werkzeug.exceptions import NotFound +from pydantic import ValidationError +from werkzeug.exceptions import NotFound, UnprocessableEntity +from controllers.common.schema import query_params_from_model from controllers.openapi import openapi_ns from controllers.openapi._models import ( - MAX_PAGE_LIMIT, AccountPayload, AccountResponse, PaginationEnvelope, RevokeResponse, + SessionListQuery, SessionListResponse, SessionRow, WorkspacePayload, @@ -70,13 +72,21 @@ class AccountSessionsSelfApi(Resource): @openapi_ns.route("/account/sessions") class AccountSessionsApi(Resource): + @openapi_ns.doc(params=query_params_from_model(SessionListQuery)) @openapi_ns.response(200, "Session list", openapi_ns.models[SessionListResponse.__name__]) @auth_router.guard(scope=Scope.FULL, allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT})) def get(self, *, auth_data: AuthData): + # Validate page/limit through the same model the contract advertises (extra='forbid', + # page>=1, 1<=limit<=MAX_PAGE_LIMIT) so the server actually enforces those bounds rather + # than silently coercing (e.g. page=0 -> empty slice). Mirrors AppDescribeQuery. + try: + query = SessionListQuery.model_validate(request.args.to_dict(flat=True)) + except ValidationError as exc: + raise UnprocessableEntity(exc.json()) ctx = get_auth_ctx() now = datetime.now(UTC) - page = int(request.args.get("page", "1")) - limit = min(int(request.args.get("limit", "100")), MAX_PAGE_LIMIT) + page = query.page + limit = query.limit all_rows = list_active_sessions(db.session, ctx, now) diff --git a/api/controllers/openapi/app_run.py b/api/controllers/openapi/app_run.py index 8ef94740c9..7b9030362a 100644 --- a/api/controllers/openapi/app_run.py +++ b/api/controllers/openapi/app_run.py @@ -15,7 +15,7 @@ from werkzeug.exceptions import BadRequest, HTTPException, InternalServerError, import services from controllers.openapi import openapi_ns from controllers.openapi._audit import emit_app_run -from controllers.openapi._models import AppRunRequest +from controllers.openapi._models import AppRunRequest, TaskStopResponse from controllers.openapi.auth.composition import auth_router from controllers.openapi.auth.data import AuthData from controllers.service_api.app.error import ( @@ -159,7 +159,7 @@ class AppRunApi(Resource): @openapi_ns.route("/apps//tasks//stop") class AppRunTaskStopApi(Resource): - @openapi_ns.response(200, "Task stopped") + @openapi_ns.response(200, "Task stopped", openapi_ns.models[TaskStopResponse.__name__]) @auth_router.guard(scope=Scope.APPS_RUN) def post(self, app_id: str, task_id: str, *, auth_data: AuthData): app_model, caller, caller_kind = auth_data.require_app_context() diff --git a/api/controllers/openapi/human_input_form.py b/api/controllers/openapi/human_input_form.py index 3c359406be..6b9d4a711e 100644 --- a/api/controllers/openapi/human_input_form.py +++ b/api/controllers/openapi/human_input_form.py @@ -17,6 +17,7 @@ from werkzeug.exceptions import BadRequest, NotFound from controllers.common.human_input import HumanInputFormSubmitPayload, stringify_form_default_values from controllers.common.schema import register_schema_models from controllers.openapi import openapi_ns +from controllers.openapi._models import FormSubmitResponse from controllers.openapi.auth.composition import auth_router from controllers.openapi.auth.data import AuthData from core.workflow.human_input_policy import HumanInputSurface, is_recipient_type_allowed_for_surface @@ -70,7 +71,7 @@ class OpenApiWorkflowHumanInputFormApi(Resource): return _jsonify_form_definition(form) @openapi_ns.expect(openapi_ns.models[HumanInputFormSubmitPayload.__name__]) - @openapi_ns.response(200, "Form submitted") + @openapi_ns.response(200, "Form submitted", openapi_ns.models[FormSubmitResponse.__name__]) @auth_router.guard(scope=Scope.APPS_RUN) def post(self, app_id: str, form_token: str, *, auth_data: AuthData): app_model, caller, caller_kind = auth_data.require_app_context() diff --git a/api/controllers/openapi/index.py b/api/controllers/openapi/index.py index a6626f9cc6..ae1780aecd 100644 --- a/api/controllers/openapi/index.py +++ b/api/controllers/openapi/index.py @@ -1,9 +1,11 @@ from flask_restx import Resource from controllers.openapi import openapi_ns +from controllers.openapi._models import HealthResponse @openapi_ns.route("/_health") class HealthApi(Resource): + @openapi_ns.response(200, "Health check", openapi_ns.models[HealthResponse.__name__]) def get(self): return {"ok": True} diff --git a/api/openapi/markdown/openapi-swagger.md b/api/openapi/markdown/openapi-swagger.md index 181d867f4f..150237b3c5 100644 --- a/api/openapi/markdown/openapi-swagger.md +++ b/api/openapi/markdown/openapi-swagger.md @@ -21,9 +21,9 @@ User-scoped operations #### GET ##### Responses -| Code | Description | -| ---- | ----------- | -| 200 | Success | +| Code | Description | Schema | +| ---- | ----------- | ------ | +| 200 | Health check | [HealthResponse](#healthresponse) | ### /_version @@ -46,6 +46,13 @@ User-scoped operations ### /account/sessions #### GET +##### Parameters + +| Name | Located in | Description | Required | Schema | +| ---- | ---------- | ----------- | -------- | ------ | +| limit | query | | No | integer | +| page | query | | No | integer | + ##### Responses | Code | Description | Schema | @@ -104,7 +111,7 @@ User-scoped operations | Name | Located in | Description | Required | Schema | | ---- | ---------- | ----------- | -------- | ------ | | app_id | path | | Yes | string | -| fields | query | | No | [ string ] | +| fields | query | | No | string | | workspace_id | query | | No | string | ##### Responses @@ -163,9 +170,9 @@ Upload a file to use as an input variable when running the app ##### Responses -| Code | Description | -| ---- | ----------- | -| 200 | Form submitted | +| Code | Description | Schema | +| ---- | ----------- | ------ | +| 200 | Form submitted | [FormSubmitResponse](#formsubmitresponse) | ### /apps/{app_id}/run @@ -211,9 +218,9 @@ Upload a file to use as an input variable when running the app ##### Responses -| Code | Description | -| ---- | ----------- | -| 200 | Task stopped | +| Code | Description | Schema | +| ---- | ----------- | ------ | +| 200 | Task stopped | [TaskStopResponse](#taskstopresponse) | ### /oauth/device/approve @@ -446,7 +453,7 @@ Empty / omitted → all blocks. Unknown member → ValidationError → 422. | Name | Type | Description | Required | | ---- | ---- | ----------- | -------- | -| fields | [ string ] | | No | +| fields | string | | No | | workspace_id | string | | No | #### AppDescribeResponse @@ -592,6 +599,23 @@ mode is a closed enum. | tenant_id | string | | No | | user_id | string | | No | +#### FormSubmitResponse + +Empty 200 body for POST /apps//form/human_input/. `extra='forbid'` +pins `additionalProperties: false` so the generated contract is an exact `{}` rather +than an under-annotated open object. + +| Name | Type | Description | Required | +| ---- | ---- | ----------- | -------- | + +#### HealthResponse + +Liveness payload for `GET /openapi/v1/_health` — no auth required. + +| Name | Type | Description | Required | +| ---- | ---- | ----------- | -------- | +| ok | boolean | | Yes | + #### HumanInputFormSubmitPayload | Name | Type | Description | Required | @@ -708,6 +732,15 @@ Meta endpoint payload for `GET /openapi/v1/_version` — no auth required. | edition | string | *Enum:* `"CLOUD"`, `"SELF_HOSTED"` | Yes | | version | string | | Yes | +#### SessionListQuery + +Pagination for GET /account/sessions. Strict (extra='forbid'). + +| Name | Type | Description | Required | +| ---- | ---- | ----------- | -------- | +| limit | integer | | No | +| page | integer | | No | + #### SessionListResponse | Name | Type | Description | Required | @@ -736,6 +769,16 @@ Meta endpoint payload for `GET /openapi/v1/_version` — no auth required. | ---- | ---- | ----------- | -------- | | name | string | | Yes | +#### TaskStopResponse + +200 body for POST /apps//tasks//stop. The handler always returns +{"result": "success"}, so `result` is required (no default) — the generated contract +types it as a required `'success'` rather than an optional field. + +| Name | Type | Description | Required | +| ---- | ---- | ----------- | -------- | +| result | string | | Yes | + #### UsageInfo | Name | Type | Description | Required | diff --git a/api/tests/unit_tests/controllers/openapi/test_account.py b/api/tests/unit_tests/controllers/openapi/test_account.py index f73dc5c0cc..5bab035e45 100644 --- a/api/tests/unit_tests/controllers/openapi/test_account.py +++ b/api/tests/unit_tests/controllers/openapi/test_account.py @@ -1,10 +1,15 @@ """User-scoped identity + session endpoints under /openapi/v1/account.""" import builtins +import sys +import uuid +from types import SimpleNamespace +from unittest.mock import MagicMock import pytest from flask import Flask from flask.views import MethodView +from werkzeug.exceptions import UnprocessableEntity from controllers.openapi import bp as openapi_bp from controllers.openapi.account import ( @@ -13,6 +18,8 @@ from controllers.openapi.account import ( AccountSessionsApi, AccountSessionsSelfApi, ) +from controllers.openapi.auth.data import AuthData +from libs.oauth_bearer import Scope, TokenType if not hasattr(builtins, "MethodView"): builtins.MethodView = MethodView # type: ignore[attr-defined] @@ -138,3 +145,74 @@ def test_subject_match_for_external_sso_filters_by_email_and_issuer(): assert "subject_email" in rendered assert "subject_issuer" in rendered assert "account_id IS NULL" in rendered + + +# --- GET /account/sessions query validation (the handler routes ?page/?limit through +# SessionListQuery so the server enforces the bounds the contract advertises). The auth ctx and +# DB read are stubbed so these exercise only the validation + paging path; __wrapped__ skips the +# auth guard, which is covered separately in auth/. --- + +_ACCOUNT_MOD = "controllers.openapi.account" + + +def _session_auth_data() -> AuthData: + return AuthData( + token_type=TokenType.OAUTH_ACCOUNT, + account_id=uuid.uuid4(), + token_hash="test", + token_id=uuid.uuid4(), + scopes=frozenset({Scope.FULL}), + required_scope=Scope.FULL, + allowed_roles=None, + ) + + +def _stub_session_deps(monkeypatch, rows): + mod = sys.modules[_ACCOUNT_MOD] + monkeypatch.setattr(mod, "get_auth_ctx", lambda: SimpleNamespace()) + monkeypatch.setattr(mod, "list_active_sessions", lambda *args, **kwargs: rows) + monkeypatch.setattr(mod, "db", MagicMock()) + + +def test_sessions_list_valid_query_parses_page_and_limit(app, monkeypatch): + """A valid ?page&limit round-trips through SessionListQuery into the response envelope.""" + api = AccountSessionsApi() + _stub_session_deps(monkeypatch, []) + with app.test_request_context("/openapi/v1/account/sessions?page=2&limit=5"): + body, status = api.get.__wrapped__(api, auth_data=_session_auth_data()) + assert status == 200 + assert body["page"] == 2 + assert body["limit"] == 5 + assert body["total"] == 0 + assert body["data"] == [] + + +def test_sessions_list_defaults_when_query_omitted(app, monkeypatch): + """No query → the model's defaults (page=1, limit=100) drive the envelope.""" + api = AccountSessionsApi() + _stub_session_deps(monkeypatch, []) + with app.test_request_context("/openapi/v1/account/sessions"): + body, status = api.get.__wrapped__(api, auth_data=_session_auth_data()) + assert status == 200 + assert body["page"] == 1 + assert body["limit"] == 100 + + +@pytest.mark.parametrize( + "query", + [ + "page=0", # below ge=1 (previously coerced to a silent empty slice) + "page=-3", + "limit=0", # below ge=1 + "limit=999", # above le=MAX_PAGE_LIMIT + "page=abc", # not an integer (previously a 500) + "foo=bar", # extra='forbid' + ], +) +def test_sessions_list_rejects_out_of_bounds_query(app, monkeypatch, query): + """Out-of-range / unknown query params raise 422 instead of being silently coerced.""" + api = AccountSessionsApi() + _stub_session_deps(monkeypatch, []) + with app.test_request_context(f"/openapi/v1/account/sessions?{query}"): + with pytest.raises(UnprocessableEntity): + api.get.__wrapped__(api, auth_data=_session_auth_data()) diff --git a/cli/package.json b/cli/package.json index 6689aa80d8..c4c3cb468f 100644 --- a/cli/package.json +++ b/cli/package.json @@ -58,6 +58,9 @@ "dependencies": { "@dify/contracts": "workspace:*", "@napi-rs/keyring": "catalog:", + "@orpc/client": "catalog:", + "@orpc/contract": "catalog:", + "@orpc/openapi-client": "catalog:", "cli-table3": "catalog:", "eventsource-parser": "catalog:", "js-yaml": "catalog:", diff --git a/cli/src/api/account-sessions.ts b/cli/src/api/account-sessions.ts index 4055ca1dd2..667deec9a2 100644 --- a/cli/src/api/account-sessions.ts +++ b/cli/src/api/account-sessions.ts @@ -1,24 +1,24 @@ import type { SessionListResponse } from '@dify/contracts/api/openapi/types.gen' +import type { OpenApiClient } from '@/http/orpc' import type { HttpClient } from '@/http/types' +import { createOpenApiClient } from '@/http/orpc' export class AccountSessionsClient { - private readonly http: HttpClient + private readonly orpc: OpenApiClient constructor(http: HttpClient) { - this.http = http + this.orpc = createOpenApiClient(http) } async list(q?: { page?: number, limit?: number }): Promise { - return this.http.get('account/sessions', { - searchParams: { page: q?.page, limit: q?.limit }, - }) + return this.orpc.account.sessions.get({ query: { page: q?.page, limit: q?.limit } }) } async revoke(sessionId: string): Promise { - await this.http.delete(`account/sessions/${encodeURIComponent(sessionId)}`) + await this.orpc.account.sessions.bySessionId.delete({ params: { session_id: sessionId } }) } async revokeSelf(): Promise { - await this.http.delete('account/sessions/self') + await this.orpc.account.sessions.self.delete() } } diff --git a/cli/src/api/account.ts b/cli/src/api/account.ts index 9e45b7c68c..39f61d63bc 100644 --- a/cli/src/api/account.ts +++ b/cli/src/api/account.ts @@ -1,14 +1,16 @@ import type { AccountResponse } from '@dify/contracts/api/openapi/types.gen' +import type { OpenApiClient } from '@/http/orpc' import type { HttpClient } from '@/http/types' +import { createOpenApiClient } from '@/http/orpc' export class AccountClient { - private readonly http: HttpClient + private readonly orpc: OpenApiClient constructor(http: HttpClient) { - this.http = http + this.orpc = createOpenApiClient(http) } async get(): Promise { - return this.http.get('account') + return this.orpc.account.get() } } diff --git a/cli/src/api/app-run.ts b/cli/src/api/app-run.ts index 5daa948b6b..1cb64057f8 100644 --- a/cli/src/api/app-run.ts +++ b/cli/src/api/app-run.ts @@ -1,5 +1,7 @@ +import type { OpenApiClient } from '@/http/orpc' import type { SseEvent } from '@/http/sse' import type { HttpClient } from '@/http/types' +import { createOpenApiClient } from '@/http/orpc' import { parseSSE } from '@/http/sse' import { normalizeDifyStream } from '@/http/sse-dify' @@ -36,9 +38,14 @@ export type StreamOptions = { export class AppRunClient { private readonly http: HttpClient + private readonly orpc: OpenApiClient constructor(http: HttpClient) { this.http = http + // Mixed class (SPEC §4.4): runStream / reconnectStream are SSE and stay on the raw + // `http.stream` facade; stopTask / submitHumanInput are plain JSON and go through the + // generated oRPC contract. Both facades share this one transport. + this.orpc = createOpenApiClient(http) } async runStream( @@ -59,9 +66,8 @@ export class AppRunClient { } async stopTask(appId: string, taskId: string): Promise { - await this.http.post(`apps/${encodeURIComponent(appId)}/tasks/${encodeURIComponent(taskId)}/stop`, { - json: {}, - timeoutMs: 30_000, + await this.orpc.apps.byAppId.tasks.byTaskId.stop.post({ + params: { app_id: appId, task_id: taskId }, }) } @@ -71,10 +77,10 @@ export class AppRunClient { action: string, inputs: Record, ): Promise { - await this.http.post( - `apps/${encodeURIComponent(appId)}/form/human_input/${encodeURIComponent(formToken)}`, - { json: { action, inputs }, timeoutMs: 30_000 }, - ) + await this.orpc.apps.byAppId.form.humanInput.byFormToken.post({ + params: { app_id: appId, form_token: formToken }, + body: { action, inputs }, + }) } async reconnectStream( diff --git a/cli/src/api/apps.ts b/cli/src/api/apps.ts index 06da5c72be..5932dd936b 100644 --- a/cli/src/api/apps.ts +++ b/cli/src/api/apps.ts @@ -1,5 +1,7 @@ import type { AppDescribeResponse, AppListResponse } from '@dify/contracts/api/openapi/types.gen' +import type { OpenApiClient } from '@/http/orpc' import type { HttpClient } from '@/http/types' +import { createOpenApiClient } from '@/http/orpc' export type ListQuery = { readonly workspaceId: string @@ -11,15 +13,15 @@ export type ListQuery = { } export class AppsClient { - private readonly http: HttpClient + private readonly orpc: OpenApiClient constructor(http: HttpClient) { - this.http = http + this.orpc = createOpenApiClient(http) } async list(q: ListQuery): Promise { - return this.http.get('apps', { - searchParams: { + return this.orpc.apps.get({ + query: { workspace_id: q.workspaceId, page: q.page ?? 1, limit: q.limit ?? 20, @@ -31,9 +33,12 @@ export class AppsClient { } async describe(appId: string, workspaceId: string, fields?: readonly string[]): Promise { - return this.http.get(`apps/${encodeURIComponent(appId)}/describe`, { - searchParams: { + return this.orpc.apps.byAppId.describe.get({ + params: { app_id: appId }, + query: { workspace_id: workspaceId, + // The backend parses a comma-separated string (validator splits on ','); the contract + // types `fields` as a string accordingly, so join here rather than send an array. fields: fields !== undefined && fields.length > 0 ? fields.join(',') : undefined, }, }) diff --git a/cli/src/api/members.ts b/cli/src/api/members.ts index 1ca97db618..7b1f80c08f 100644 --- a/cli/src/api/members.ts +++ b/cli/src/api/members.ts @@ -5,40 +5,41 @@ import type { MemberListResponse, MemberRoleUpdatePayload, } from '@dify/contracts/api/openapi/types.gen' +import type { OpenApiClient } from '@/http/orpc' import type { HttpClient } from '@/http/types' +import { createOpenApiClient } from '@/http/orpc' /** - * Thin client for /openapi/v1/workspaces//members. + * Thin client for /openapi/v1/workspaces//members, over the generated oRPC contract. * - * Errors are surfaced as BaseError via classifyResponse on non-2xx - * (400/403/404/422). The CLI's AuthedCommand base layer maps those to - * user-visible messages — clients never swallow status codes here. + * Non-2xx (400/403/404/422) surface as BaseError — the oRPC client maps them at the transport + * seam. The CLI's AuthedCommand base layer renders those for the user; clients never swallow codes. */ export class MembersClient { - private readonly http: HttpClient + private readonly orpc: OpenApiClient constructor(http: HttpClient) { - this.http = http + this.orpc = createOpenApiClient(http) } async list(workspaceId: string, q?: { page?: number, limit?: number }): Promise { - return this.http.get( - `workspaces/${encodeURIComponent(workspaceId)}/members`, - { searchParams: { page: q?.page, limit: q?.limit } }, - ) + return this.orpc.workspaces.byWorkspaceId.members.get({ + params: { workspace_id: workspaceId }, + query: { page: q?.page, limit: q?.limit }, + }) } async invite(workspaceId: string, payload: MemberInvitePayload): Promise { - return this.http.post( - `workspaces/${encodeURIComponent(workspaceId)}/members`, - { json: payload }, - ) + return this.orpc.workspaces.byWorkspaceId.members.post({ + params: { workspace_id: workspaceId }, + body: payload, + }) } async remove(workspaceId: string, memberId: string): Promise { - return this.http.delete( - `workspaces/${encodeURIComponent(workspaceId)}/members/${encodeURIComponent(memberId)}`, - ) + return this.orpc.workspaces.byWorkspaceId.members.byMemberId.delete({ + params: { workspace_id: workspaceId, member_id: memberId }, + }) } async updateRole( @@ -46,9 +47,9 @@ export class MembersClient { memberId: string, payload: MemberRoleUpdatePayload, ): Promise { - return this.http.put( - `workspaces/${encodeURIComponent(workspaceId)}/members/${encodeURIComponent(memberId)}/role`, - { json: payload }, - ) + return this.orpc.workspaces.byWorkspaceId.members.byMemberId.role.put({ + params: { workspace_id: workspaceId, member_id: memberId }, + body: payload, + }) } } diff --git a/cli/src/api/meta.ts b/cli/src/api/meta.ts index ed651f63df..6e9fdfdf20 100644 --- a/cli/src/api/meta.ts +++ b/cli/src/api/meta.ts @@ -1,19 +1,22 @@ import type { ServerVersionResponse } from '@dify/contracts/api/openapi/types.gen' +import type { OpenApiClient } from '@/http/orpc' import type { HttpClient } from '@/http/types' +import { createOpenApiClient } from '@/http/orpc' // Used by every /_version probe call site (the version command and the // per-command auto-nudge). Both must construct their HTTP client with this // timeout + retryAttempts: 0, otherwise the default 30s/3-retry budget kicks in. +// The oRPC client below inherits that http instance's policy, so the probe timeout still holds. export const META_PROBE_TIMEOUT_MS = 2000 export class MetaClient { - private readonly http: HttpClient + private readonly orpc: OpenApiClient constructor(http: HttpClient) { - this.http = http + this.orpc = createOpenApiClient(http) } async serverVersion(): Promise { - return this.http.get('_version') + return this.orpc.version.get() } } diff --git a/cli/src/api/workspaces.test.ts b/cli/src/api/workspaces.test.ts index d2cbd0da21..b9b5fb6347 100644 --- a/cli/src/api/workspaces.test.ts +++ b/cli/src/api/workspaces.test.ts @@ -5,7 +5,10 @@ import { afterEach, describe, expect, it } from 'vitest' import { isHttpClientError } from '@/errors/base' import { WorkspacesClient } from './workspaces.js' -// WorkspacesClient.switch is covered in members.test.ts; this file covers list(). +// WorkspacesClient.switch is covered in members.test.ts; this file covers list(), which now +// routes through the generated oRPC contract (OpenAPILink over http.request). The happy path +// asserts the on-the-wire GET /openapi/v1/workspaces is unchanged; the 401 case asserts the +// oRPC client surfaces it as the CLI's classified HttpClientError. function makeClient(host: string): WorkspacesClient { return new WorkspacesClient(testHttpClient(host, 'dfoa_test')) diff --git a/cli/src/api/workspaces.ts b/cli/src/api/workspaces.ts index 7325f8e2cb..3ef587b574 100644 --- a/cli/src/api/workspaces.ts +++ b/cli/src/api/workspaces.ts @@ -1,15 +1,20 @@ import type { WorkspaceDetailResponse, WorkspaceListResponse } from '@dify/contracts/api/openapi/types.gen' +import type { OpenApiClient } from '@/http/orpc' import type { HttpClient } from '@/http/types' +import { createOpenApiClient } from '@/http/orpc' export class WorkspacesClient { - private readonly http: HttpClient + private readonly orpc: OpenApiClient constructor(http: HttpClient) { - this.http = http + // oRPC client over the same transport (UA+bearer / retry / timeout / error-map) — SPEC §4.4: + // one transport, a contract facade. Both methods are standard unary JSON, so both go through + // the generated contract. + this.orpc = createOpenApiClient(http) } async list(): Promise { - return this.http.get('workspaces') + return this.orpc.workspaces.get() } /** @@ -22,6 +27,6 @@ export class WorkspacesClient { * server's state. */ async switch(workspaceId: string): Promise { - return this.http.post(`workspaces/${encodeURIComponent(workspaceId)}/switch`) + return this.orpc.workspaces.byWorkspaceId.switch.post({ params: { workspace_id: workspaceId } }) } } diff --git a/cli/src/http/client.test.ts b/cli/src/http/client.test.ts index c7954e8bc2..fbde1ecdca 100644 --- a/cli/src/http/client.test.ts +++ b/cli/src/http/client.test.ts @@ -591,3 +591,147 @@ describe('hook semantics', () => { await expect(client.get('workspaces')).rejects.toThrow('hook boom on error') }) }) + +// The low-level request() entrypoint: oRPC's OpenAPILink builds an absolute-URL Request and +// calls a fetch-shaped fn. These tests pin the three pitfalls (clone-on-retry / signal merge / +// skip-baseURL) plus UA+bearer injection and the raw-Response-on-error contract oRPC relies on. +describe('request() entrypoint (oRPC OpenAPILink socket)', () => { + it('sends to the Request URL verbatim, skipping the client baseURL', async () => { + const mock = await startMock() + try { + // Deliberately wrong baseURL: if request() re-joined it via joinURL, this would miss. + const client = createHttpClient({ baseURL: 'http://wrong.invalid/openapi/v1/', bearer: 'dfoa_test' }) + const res = await client.request(new Request(`${base(mock.url)}workspaces`)) + expect(res.status).toBe(200) + const body = await res.json() as { workspaces: unknown[] } + expect(body.workspaces).toHaveLength(2) + } + finally { + await mock.stop() + } + }) + + it('injects UA + bearer that the pre-built Request did not carry', async () => { + const mock = await startMock() + try { + let auth: string | null = null + let ua: string | null = null + const client = createHttpClient({ + baseURL: base(mock.url), + bearer: 'dfoa_test', + userAgent: 'difyctl/req-test', + hooks: { + onRequest: ({ request }) => { + auth = request.headers.get('authorization') + ua = request.headers.get('user-agent') + }, + }, + }) + await client.request(new Request(`${base(mock.url)}workspaces`)) + expect(auth).toBe('Bearer dfoa_test') + expect(ua).toBe('difyctl/req-test') + } + finally { + await mock.stop() + } + }) + + it('returns the raw Response on 4xx and does NOT throw (oRPC reads the body + maps errors)', async () => { + const mock = await startMock() + try { + mock.setScenario('auth-expired') + const client = createHttpClient({ baseURL: base(mock.url), bearer: 'dfoa_test' }) + const res = await client.request(new Request(`${base(mock.url)}workspaces`)) + expect(res.ok).toBe(false) + expect(res.status).toBe(401) + } + finally { + await mock.stop() + } + }) + + it('clones the Request per attempt so a retried body replays intact', async () => { + let hits = 0 + let secondBody = '' + const stub = await startStub((req, res) => { + hits++ + let raw = '' + req.on('data', (c) => { + raw += String(c) + }) + req.on('end', () => { + if (hits === 1) { + res.writeHead(503).end() + return + } + secondBody = raw + res.writeHead(200, { 'content-type': 'application/json' }).end('{"ok":true}') + }) + }) + try { + const client = createHttpClient({ baseURL: stub.url, bearer: 'dfoa_test', retryAttempts: 2, timeoutMs: 5_000 }) + // PUT is in RETRY_METHODS and carries a body — proves clone-on-retry replays it. + const res = await client.request(new Request(`${stub.url}/x`, { + method: 'PUT', + body: JSON.stringify({ a: 1 }), + headers: { 'content-type': 'application/json' }, + })) + expect(res.status).toBe(200) + expect(hits).toBe(2) + expect(secondBody).toBe('{"a":1}') + } + finally { + await stub.stop() + } + }) + + it('applies the client-default timeout on top of the Request signal', async () => { + const stub = await startStub(() => { /* never responds */ }) + try { + const client = createHttpClient({ baseURL: stub.url, bearer: 'dfoa_test', timeoutMs: 100, retryAttempts: 0 }) + const ac = new AbortController() // oRPC's own signal, never aborted + await expect(client.request(new Request(`${stub.url}/x`, { signal: ac.signal }))) + .rejects + .toBeDefined() + } + finally { + await stub.stop() + } + }) + + it('does not retry when the Request (oRPC/user) signal aborts', async () => { + let hits = 0 + const stub = await startStub(() => { + hits++ // never responds + }) + try { + const ac = new AbortController() + const client = createHttpClient({ baseURL: stub.url, bearer: 'dfoa_test', retryAttempts: 3, timeoutMs: 5_000 }) + const pending = client.request(new Request(`${stub.url}/x`, { method: 'GET', signal: ac.signal })) + setTimeout(() => ac.abort(), 50) + await expect(pending).rejects.toBeDefined() + expect(hits).toBe(1) + } + finally { + await stub.stop() + } + }) + + it('gives each retried attempt a fresh timeout budget', async () => { + let hits = 0 + const stub = await startStub(() => { + hits++ // never responds + }) + try { + const client = createHttpClient({ baseURL: stub.url, bearer: 'dfoa_test', timeoutMs: 100, retryAttempts: 2 }) + // GET is retryable; each attempt must mint its own 100ms timeout, so all 3 fire. If the + // timeout signal were hoisted out of the retry loop, attempt 0's expired signal would + // short-circuit attempts 1-2 and hits would be 1. + await expect(client.request(new Request(`${stub.url}/x`))).rejects.toBeDefined() + expect(hits).toBe(3) + } + finally { + await stub.stop() + } + }, 30_000) +}) diff --git a/cli/src/http/client.ts b/cli/src/http/client.ts index 6143a519d6..f099b01225 100644 --- a/cli/src/http/client.ts +++ b/cli/src/http/client.ts @@ -82,11 +82,12 @@ async function runHooks(hooks: readonly Hook[], ctx: FetchContext): Promise 0 ? AbortSignal.timeout(effectiveTimeoutMs) : undefined - const userSignal = opts.signal if (timeoutSignal === undefined) return userSignal @@ -102,7 +103,17 @@ function mergeHeaders(input: HeadersInit | undefined, contentType: string | unde return headers } -async function dispatch(state: ClientState, path: string, opts: RequestOptions, attempt: number, throwOnErrorDefault: boolean): Promise { +type BuiltRequest = { + readonly request: Request + readonly resolved: ResolvedOptions + readonly effectiveTimeoutMs: number | undefined + readonly userSignal: AbortSignal | undefined +} + +// Path-keyed constructor: turns (path, opts) into a Request. The Request is built +// WITHOUT a signal — execute() supplies a fresh per-attempt signal via fetch's +// init.signal (which overrides any signal carried on the Request). +function buildRequest(state: ClientState, path: string, opts: RequestOptions, throwOnErrorDefault: boolean): BuiltRequest { const method: HttpMethod = opts.method ?? 'GET' const effectiveTimeoutMs = opts.timeoutMs !== undefined ? (opts.timeoutMs > 0 ? opts.timeoutMs : undefined) @@ -114,9 +125,7 @@ async function dispatch(state: ClientState, path: string, opts: RequestOptions, const headers = mergeHeaders(opts.headers, contentType) const url = appendSearchParams(joinURL(state.baseURL, path), opts.searchParams) - const signal = buildSignal(opts, effectiveTimeoutMs) - - const request = new Request(url, { method, headers, body, signal }) + const request = new Request(url, { method, headers, body }) const resolved: ResolvedOptions = { method, headers, @@ -125,73 +134,100 @@ async function dispatch(state: ClientState, path: string, opts: RequestOptions, retryAttempts: effectiveRetryAttempts, throwOnError, } - const ctx: FetchContext = { - request, - options: resolved, - attempt, - meta: new Map(), - } + return { request, resolved, effectiveTimeoutMs, userSignal: opts.signal } +} - await runHooks(state.hooks.onRequest, ctx) +// The shared transport engine: hooks -> fetch -> retry -> error-map. Accepts an +// already-built Request, so both the path entrypoints (via buildRequest) and the +// low-level `request` entrypoint (oRPC's pre-built Request) share one retry/timeout +// /error policy. +async function execute( + state: ClientState, + request: Request, + resolved: ResolvedOptions, + effectiveTimeoutMs: number | undefined, + userSignal: AbortSignal | undefined, +): Promise { + const { method, retryAttempts: effectiveRetryAttempts, throwOnError } = resolved - const init: RequestInit & { dispatcher?: unknown, verbose?: boolean } = { signal } - if (state.dispatcher !== undefined) - init.dispatcher = state.dispatcher - if (isVerbose()) - init.verbose = true + for (let attempt = 0; ; attempt++) { + // Fresh timeout budget per attempt, merged with the persistent user/oRPC signal. + // Mirrors the old recursion that re-ran buildSignal() on every retry. Keep this + // INSIDE the loop — hoisting it would leak attempt 0's timeout into later attempts. + const signal = mergeSignal(userSignal, effectiveTimeoutMs) - try { - ctx.response = await fetch(ctx.request, init) - } - catch (err) { - ctx.error = err - // Snapshot the abort cause before onRequestError hooks rewrite ctx.error into BaseError. - const userAborted = opts.signal?.aborted === true - await runHooks(state.hooks.onRequestError, ctx) + // clone-on-retry: the first fetch consumes the Request body, so any attempt that + // may still be retried sends a clone and keeps `request` as the pristine replay copy. + const sendable = attempt < effectiveRetryAttempts ? request.clone() : request - // User aborts (ctrl+C) must never retry. Timeouts and other transport errors fall - // through to shouldRetry, which enforces the method allowlist. - if (!userAborted && attempt < effectiveRetryAttempts && shouldRetry(ctx.error, ctx)) { - state.logger?.({ phase: 'retry', method, url: redactBearer(request.url), attempt: attempt + 1 }) - const delay = backoffDelay(attempt + 1) - if (delay > 0) - await new Promise(resolve => setTimeout(resolve, delay)) - return dispatch(state, path, opts, attempt + 1, throwOnErrorDefault) + const ctx: FetchContext = { + request: sendable, + options: resolved, + attempt, + meta: new Map(), } - const finalErr = ctx.error - if (finalErr instanceof Error && typeof Error.captureStackTrace === 'function') - Error.captureStackTrace(finalErr, dispatch) - throw finalErr - } + await runHooks(state.hooks.onRequest, ctx) - await runHooks(state.hooks.onResponse, ctx) + const init: RequestInit & { dispatcher?: unknown, verbose?: boolean } = { signal } + if (state.dispatcher !== undefined) + init.dispatcher = state.dispatcher + if (isVerbose()) + init.verbose = true - const res = ctx.response - if (!res.ok) { - if (attempt < effectiveRetryAttempts && shouldRetry(res, ctx)) { - state.logger?.({ phase: 'retry', method, url: redactBearer(request.url), attempt: attempt + 1 }) - // Drain the discarded error body so undici can release the socket back to its - // pool instead of holding the connection open until keep-alive timeout / GC. - await res.body?.cancel().catch(() => {}) - const delay = backoffDelay(attempt + 1) - if (delay > 0) - await new Promise(resolve => setTimeout(resolve, delay)) - return dispatch(state, path, opts, attempt + 1, throwOnErrorDefault) + try { + ctx.response = await fetch(ctx.request, init) } + catch (err) { + ctx.error = err + // Snapshot the abort cause before onRequestError hooks rewrite ctx.error into BaseError. + const userAborted = userSignal?.aborted === true + await runHooks(state.hooks.onRequestError, ctx) - ctx.error = await classifyResponse(request, res) - await runHooks(state.hooks.onResponseError, ctx) + // User aborts (ctrl+C) must never retry. Timeouts and other transport errors fall + // through to shouldRetry, which enforces the method allowlist. + if (!userAborted && attempt < effectiveRetryAttempts && shouldRetry(ctx.error, ctx)) { + state.logger?.({ phase: 'retry', method, url: redactBearer(ctx.request.url), attempt: attempt + 1 }) + const delay = backoffDelay(attempt + 1) + if (delay > 0) + await new Promise(resolve => setTimeout(resolve, delay)) + continue + } - if (throwOnError) { const finalErr = ctx.error if (finalErr instanceof Error && typeof Error.captureStackTrace === 'function') - Error.captureStackTrace(finalErr, dispatch) + Error.captureStackTrace(finalErr, execute) throw finalErr } - } - return res + await runHooks(state.hooks.onResponse, ctx) + + const res = ctx.response + if (!res.ok) { + if (attempt < effectiveRetryAttempts && shouldRetry(res, ctx)) { + state.logger?.({ phase: 'retry', method, url: redactBearer(ctx.request.url), attempt: attempt + 1 }) + // Drain the discarded error body so undici can release the socket back to its + // pool instead of holding the connection open until keep-alive timeout / GC. + await res.body?.cancel().catch(() => {}) + const delay = backoffDelay(attempt + 1) + if (delay > 0) + await new Promise(resolve => setTimeout(resolve, delay)) + continue + } + + ctx.error = await classifyResponse(ctx.request, res) + await runHooks(state.hooks.onResponseError, ctx) + + if (throwOnError) { + const finalErr = ctx.error + if (finalErr instanceof Error && typeof Error.captureStackTrace === 'function') + Error.captureStackTrace(finalErr, execute) + throw finalErr + } + } + + return res + } } // 204/205 and empty 2xx bodies carry no JSON. Resolve to `undefined` instead of @@ -209,17 +245,19 @@ export function createHttpClient(opts: ClientOptions): HttpClient { const typedCall = async (method: HttpMethod, path: string, callOpts?: RequestOptions): Promise => { const finalOpts: RequestOptions = { ...callOpts, method } - const res = await dispatch(state, path, finalOpts, 0, true) + const built = buildRequest(state, path, finalOpts, true) + const res = await execute(state, built.request, built.resolved, built.effectiveTimeoutMs, built.userSignal) return parseJsonBody(res) } const rawFetch = (path: string, callOpts?: RequestOptions): Promise => { const finalOpts: RequestOptions = { ...callOpts, method: callOpts?.method ?? 'GET' } - return dispatch(state, path, finalOpts, 0, false) + const built = buildRequest(state, path, finalOpts, false) + return execute(state, built.request, built.resolved, built.effectiveTimeoutMs, built.userSignal) } const streamFetch = (path: string, callOpts?: RequestOptions): Promise => { - // SSE bodies must not be aborted by a request-level timeout — `0` is the dispatch + // SSE bodies must not be aborted by a request-level timeout — `0` is the buildRequest // sentinel for "no timeout" and also overrides the client default. const finalOpts: RequestOptions = { ...callOpts, @@ -227,12 +265,34 @@ export function createHttpClient(opts: ClientOptions): HttpClient { retryAttempts: 0, timeoutMs: 0, } - return dispatch(state, path, finalOpts, 0, false) + const built = buildRequest(state, path, finalOpts, false) + return execute(state, built.request, built.resolved, built.effectiveTimeoutMs, built.userSignal) + } + + // Low-level entrypoint for oRPC's OpenAPILink: executes an already-built, absolute-URL + // Request through the same transport (UA+bearer hooks, retry, timeout, error-map) while + // skipping joinURL. Policy comes from the client instance defaults — there is no per-call + // override, so this stays a drop-in for OpenAPILink's `(req, init) => Promise`. + // Returns the raw Response for every status; the oRPC fetch wrapper (orpc.ts) inspects the + // status and raises classifyResponse for non-2xx, so error mapping stays in one place. + const requestFetch = (req: Request, init?: RequestInit): Promise => { + const method = req.method.toUpperCase() as HttpMethod + const resolved: ResolvedOptions = { + method, + headers: req.headers, + body: undefined, + timeoutMs: state.defaultTimeoutMs, + retryAttempts: state.defaultRetryAttempts, + throwOnError: false, + } + const userSignal = init?.signal ?? req.signal + return execute(state, req, resolved, state.defaultTimeoutMs, userSignal) } const extend = (overrides: Partial): HttpClient => createHttpClient({ ...state.originalOptions, ...overrides }) return { + baseURL: state.baseURL, get: (p: string, o?: RequestOptions) => typedCall('GET', p, o), post: (p: string, o?: RequestOptions) => typedCall('POST', p, o), put: (p: string, o?: RequestOptions) => typedCall('PUT', p, o), @@ -240,6 +300,7 @@ export function createHttpClient(opts: ClientOptions): HttpClient { delete: (p: string, o?: RequestOptions) => typedCall('DELETE', p, o), fetch: rawFetch, stream: streamFetch, + request: requestFetch, extend, } } diff --git a/cli/src/http/orpc.test.ts b/cli/src/http/orpc.test.ts new file mode 100644 index 0000000000..05e9a8405f --- /dev/null +++ b/cli/src/http/orpc.test.ts @@ -0,0 +1,98 @@ +import type { StubServer } from '@test/fixtures/stub-server' +import { jsonResponder, startStubServer } from '@test/fixtures/stub-server' +import { afterEach, describe, expect, it } from 'vitest' +import { isHttpClientError } from '@/errors/base' +import { ErrorCode } from '@/errors/codes' +import { openAPIBase } from '@/util/host' +import { createHttpClient } from './client.js' +import { createOpenApiClient } from './orpc.js' + +// createOpenApiClient maps errors at the transport seams, so a migrated endpoint surfaces the +// SAME classified HttpClientError as the `this.http.*` path — Dify's message/hint, the request +// method/url, and the raw body — straight from a plain `orpc.x()` call, with no per-call wrapper. +function orpcClient(host: string) { + // retryAttempts: 0 so the 5xx case fails fast instead of burning the backoff budget. + const http = createHttpClient({ baseURL: openAPIBase(host), bearer: 'dfoa_test', retryAttempts: 0 }) + return createOpenApiClient(http) +} + +async function catchErr(run: () => Promise): Promise { + try { + await run() + return undefined + } + catch (err) { + return err + } +} + +describe('createOpenApiClient error mapping', () => { + let stub: StubServer + + afterEach(async () => { + await stub?.stop() + }) + + it('recovers Dify message + hint from a top-level 4xx envelope', async () => { + stub = await startStubServer(cap => jsonResponder(403, { message: 'no access', hint: 'ask an admin' }, cap)) + const orpc = orpcClient(stub.url) + + const caught = await catchErr(() => orpc.account.get()) + + expect(isHttpClientError(caught)).toBe(true) + if (isHttpClientError(caught)) { + expect(caught.code).toBe(ErrorCode.Server4xxOther) + expect(caught.httpStatus).toBe(403) + expect(caught.message).toBe('no access') + expect(caught.hint).toBe('ask an admin') + // Parity with the transport path: the migrated endpoint's error keeps the request + // method/url and the raw body, so formatted errors still print the `request:` line + // and the raw-response dump (not just message/hint). + expect(caught.method).toBe('GET') + expect(caught.url).toContain('/account') + expect(caught.rawResponse).toContain('no access') + } + }) + + it('recovers from a nested { error: { message, hint } } envelope and keeps the auth code on 401', async () => { + stub = await startStubServer(cap => jsonResponder(401, { error: { message: 'expired', hint: 'relogin' } }, cap)) + const orpc = orpcClient(stub.url) + + const caught = await catchErr(() => orpc.account.get()) + + expect(isHttpClientError(caught)).toBe(true) + if (isHttpClientError(caught)) { + expect(caught.code).toBe(ErrorCode.AuthExpired) + expect(caught.httpStatus).toBe(401) + expect(caught.message).toBe('expired') + expect(caught.hint).toBe('relogin') + } + }) + + it('falls back to the default auth-login hint when the body carries none', async () => { + stub = await startStubServer(cap => jsonResponder(401, { error: 'expired' }, cap)) + const orpc = orpcClient(stub.url) + + const caught = await catchErr(() => orpc.account.get()) + + expect(isHttpClientError(caught)).toBe(true) + if (isHttpClientError(caught)) { + expect(caught.code).toBe(ErrorCode.AuthExpired) + expect(caught.hint).toContain('difyctl auth login') + } + }) + + it('maps 5xx to Server5xx', async () => { + stub = await startStubServer(cap => jsonResponder(503, { message: 'down for maintenance' }, cap)) + const orpc = orpcClient(stub.url) + + const caught = await catchErr(() => orpc.account.get()) + + expect(isHttpClientError(caught)).toBe(true) + if (isHttpClientError(caught)) { + expect(caught.code).toBe(ErrorCode.Server5xx) + expect(caught.httpStatus).toBe(503) + expect(caught.message).toBe('down for maintenance') + } + }) +}) diff --git a/cli/src/http/orpc.ts b/cli/src/http/orpc.ts new file mode 100644 index 0000000000..484d2d1a61 --- /dev/null +++ b/cli/src/http/orpc.ts @@ -0,0 +1,41 @@ +import type { ContractRouterClient } from '@orpc/contract' +import type { JsonifiedClient } from '@orpc/openapi-client' +import type { HttpClient } from './types.js' +import { contract } from '@dify/contracts/api/openapi/orpc.gen' +import { createORPCClient } from '@orpc/client' +import { OpenAPILink } from '@orpc/openapi-client/fetch' +import { isBaseError, unknownError } from '@/errors/base' +import { classifyResponse } from './error-mapper.js' + +// Contract-typed oRPC client for the public OpenAPI surface. `JsonifiedClient` reshapes the +// contract types to what survives JSON transport (e.g. Date -> string), matching the wire. +export type OpenApiClient = JsonifiedClient> + +// An oRPC client routed through the CLI's HttpClient, so every call reuses the one transport +// policy (UA+bearer, retry, timeout). Errors become the CLI's model at the two transport seams, +// so call sites stay plain `this.orpc.x.y(input)` with no per-method try/catch: +// - fetch wrapper: non-2xx -> classifyResponse (identical to the `this.http.*` path), leaving +// oRPC to decode only 2xx responses; +// - link wrapper: the one residual throw (a 2xx body oRPC can't decode) -> mapOrpcError. +export function createOpenApiClient(http: HttpClient): OpenApiClient { + const link = new OpenAPILink(contract, { + url: http.baseURL, + fetch: async (req, init) => { + const res = await http.request(req, init) + if (!res.ok) + throw await classifyResponse(req, res) + return res + }, + }) + return createORPCClient({ + call: (path, input, options) => link.call(path, input, options).catch(mapOrpcError), + }) +} + +// Non-2xx and transport failures already arrive as BaseError (from the fetch wrapper / transport) +// and re-throw unchanged; the only residual is a 2xx body oRPC failed to decode. +function mapOrpcError(err: unknown): never { + if (isBaseError(err)) + throw err + throw unknownError(err instanceof Error ? err.message : String(err), err) +} diff --git a/cli/src/http/types.ts b/cli/src/http/types.ts index a702492d23..e06bbfc9fa 100644 --- a/cli/src/http/types.ts +++ b/cli/src/http/types.ts @@ -73,6 +73,9 @@ export type ClientOptions = { } export type HttpClient = { + // The resolved base URL this client was created with (e.g. openAPIBase(host)). Exposed so + // callers can build a contract/oRPC facade over the same transport without re-deriving it. + readonly baseURL: string readonly get: (path: string, opts?: RequestOptions) => Promise readonly post: (path: string, opts?: RequestOptions) => Promise readonly put: (path: string, opts?: RequestOptions) => Promise @@ -80,5 +83,8 @@ export type HttpClient = { readonly delete: (path: string, opts?: RequestOptions) => Promise readonly fetch: (path: string, opts?: RequestOptions) => Promise readonly stream: (path: string, opts?: RequestOptions) => Promise + // Low-level entrypoint for oRPC's OpenAPILink: runs a pre-built, absolute-URL Request + // through the transport (UA+bearer, retry, timeout, error-map) without re-joining baseURL. + readonly request: (req: Request, init?: RequestInit) => Promise readonly extend: (overrides: Partial) => HttpClient } diff --git a/packages/contracts/generated/api/openapi/orpc.gen.ts b/packages/contracts/generated/api/openapi/orpc.gen.ts index c837f6258e..4fa3c53614 100644 --- a/packages/contracts/generated/api/openapi/orpc.gen.ts +++ b/packages/contracts/generated/api/openapi/orpc.gen.ts @@ -10,6 +10,7 @@ import { zDeleteWorkspacesByWorkspaceIdMembersByMemberIdPath, zDeleteWorkspacesByWorkspaceIdMembersByMemberIdResponse, zGetAccountResponse, + zGetAccountSessionsQuery, zGetAccountSessionsResponse, zGetAppsByAppIdDescribePath, zGetAppsByAppIdDescribeQuery, @@ -59,16 +60,8 @@ import { zPutWorkspacesByWorkspaceIdMembersByMemberIdRoleResponse, } from './zod.gen' -/** - * Generated contract types may be inaccurate because backend OpenAPI annotations are incomplete. Do not migrate callers until the generated contract is accurate. - * - * @deprecated - */ export const get = oc .route({ - deprecated: true, - description: - 'Generated contract types may be inaccurate because backend OpenAPI annotations are incomplete. Do not migrate callers until the generated contract is accurate.', inputStructure: 'detailed', method: 'GET', operationId: 'getHealth', @@ -132,6 +125,7 @@ export const get3 = oc path: '/account/sessions', tags: ['openapi'], }) + .input(z.object({ query: zGetAccountSessionsQuery.optional() })) .output(zGetAccountSessionsResponse) export const sessions = { @@ -155,16 +149,8 @@ export const account = { sessions, } -/** - * Generated contract types may be inaccurate because backend OpenAPI annotations are incomplete. Do not migrate callers until the generated contract is accurate. - * - * @deprecated - */ export const get5 = oc .route({ - deprecated: true, - description: - 'Generated contract types may be inaccurate because backend OpenAPI annotations are incomplete. Do not migrate callers until the generated contract is accurate.', inputStructure: 'detailed', method: 'GET', operationId: 'getAppsByAppIdDescribe', @@ -226,16 +212,8 @@ export const get6 = oc .input(z.object({ params: zGetAppsByAppIdFormHumanInputByFormTokenPath })) .output(zGetAppsByAppIdFormHumanInputByFormTokenResponse) -/** - * Generated contract types may be inaccurate because backend OpenAPI annotations are incomplete. Do not migrate callers until the generated contract is accurate. - * - * @deprecated - */ export const post2 = oc .route({ - deprecated: true, - description: - 'Generated contract types may be inaccurate because backend OpenAPI annotations are incomplete. Do not migrate callers until the generated contract is accurate.', inputStructure: 'detailed', method: 'POST', operationId: 'postAppsByAppIdFormHumanInputByFormToken', @@ -309,16 +287,8 @@ export const events = { get: get7, } -/** - * Generated contract types may be inaccurate because backend OpenAPI annotations are incomplete. Do not migrate callers until the generated contract is accurate. - * - * @deprecated - */ export const post4 = oc .route({ - deprecated: true, - description: - 'Generated contract types may be inaccurate because backend OpenAPI annotations are incomplete. Do not migrate callers until the generated contract is accurate.', inputStructure: 'detailed', method: 'POST', operationId: 'postAppsByAppIdTasksByTaskIdStop', diff --git a/packages/contracts/generated/api/openapi/types.gen.ts b/packages/contracts/generated/api/openapi/types.gen.ts index 197c35d16e..be36ccafd1 100644 --- a/packages/contracts/generated/api/openapi/types.gen.ts +++ b/packages/contracts/generated/api/openapi/types.gen.ts @@ -32,7 +32,7 @@ export type AppDescribeInfo = { } export type AppDescribeQuery = { - fields?: Array | null + fields?: string workspace_id?: string | null } @@ -161,6 +161,14 @@ export type FileResponse = { user_id?: string | null } +export type FormSubmitResponse = { + [key: string]: never +} + +export type HealthResponse = { + ok: boolean +} + export type HumanInputFormSubmitPayload = { action: string inputs: { @@ -245,6 +253,11 @@ export type ServerVersionResponse = { version: string } +export type SessionListQuery = { + limit?: number + page?: number +} + export type SessionListResponse = { data: Array has_more: boolean @@ -267,6 +280,10 @@ export type TagItem = { name: string } +export type TaskStopResponse = { + result: string +} + export type UsageInfo = { completion_tokens?: number prompt_tokens?: number @@ -323,9 +340,7 @@ export type GetHealthData = { } export type GetHealthResponses = { - 200: { - [key: string]: unknown - } + 200: HealthResponse } export type GetHealthResponse = GetHealthResponses[keyof GetHealthResponses] @@ -359,7 +374,10 @@ export type GetAccountResponse = GetAccountResponses[keyof GetAccountResponses] export type GetAccountSessionsData = { body?: never path?: never - query?: never + query?: { + limit?: number + page?: number + } url: '/account/sessions' } @@ -426,7 +444,7 @@ export type GetAppsByAppIdDescribeData = { app_id: string } query?: { - fields?: Array + fields?: string workspace_id?: string } url: '/apps/{app_id}/describe' @@ -503,9 +521,7 @@ export type PostAppsByAppIdFormHumanInputByFormTokenData = { } export type PostAppsByAppIdFormHumanInputByFormTokenResponses = { - 200: { - [key: string]: unknown - } + 200: FormSubmitResponse } export type PostAppsByAppIdFormHumanInputByFormTokenResponse @@ -559,9 +575,7 @@ export type PostAppsByAppIdTasksByTaskIdStopData = { } export type PostAppsByAppIdTasksByTaskIdStopResponses = { - 200: { - [key: string]: unknown - } + 200: TaskStopResponse } export type PostAppsByAppIdTasksByTaskIdStopResponse diff --git a/packages/contracts/generated/api/openapi/zod.gen.ts b/packages/contracts/generated/api/openapi/zod.gen.ts index cd2072b8ef..ca09b116dd 100644 --- a/packages/contracts/generated/api/openapi/zod.gen.ts +++ b/packages/contracts/generated/api/openapi/zod.gen.ts @@ -19,7 +19,7 @@ export const zAccountPayload = z.object({ * Empty / omitted → all blocks. Unknown member → ValidationError → 422. */ export const zAppDescribeQuery = z.object({ - fields: z.array(z.string()).nullish(), + fields: z.string().optional(), workspace_id: z.string().nullish(), }) @@ -141,6 +141,24 @@ export const zFileResponse = z.object({ user_id: z.string().nullish(), }) +/** + * FormSubmitResponse + * + * Empty 200 body for POST /apps//form/human_input/. `extra='forbid'` + * pins `additionalProperties: false` so the generated contract is an exact `{}` rather + * than an under-annotated open object. + */ +export const zFormSubmitResponse = z.record(z.string(), z.never()) + +/** + * HealthResponse + * + * Liveness payload for `GET /openapi/v1/_health` — no auth required. + */ +export const zHealthResponse = z.object({ + ok: z.boolean(), +}) + export const zJsonValue = z.unknown() /** @@ -247,6 +265,16 @@ export const zServerVersionResponse = z.object({ version: z.string(), }) +/** + * SessionListQuery + * + * Pagination for GET /account/sessions. Strict (extra='forbid'). + */ +export const zSessionListQuery = z.object({ + limit: z.int().gte(1).lte(200).optional().default(100), + page: z.int().gte(1).optional().default(1), +}) + /** * SessionRow */ @@ -351,6 +379,17 @@ export const zPermittedExternalAppsListResponse = z.object({ total: z.int(), }) +/** + * TaskStopResponse + * + * 200 body for POST /apps//tasks//stop. The handler always returns + * {"result": "success"}, so `result` is required (no default) — the generated contract + * types it as a required `'success'` rather than an optional field. + */ +export const zTaskStopResponse = z.object({ + result: z.string(), +}) + /** * UsageInfo */ @@ -436,9 +475,9 @@ export const zWorkspaceListResponse = z.object({ }) /** - * Success + * Health check */ -export const zGetHealthResponse = z.record(z.string(), z.unknown()) +export const zGetHealthResponse = zHealthResponse /** * Server version @@ -450,6 +489,11 @@ export const zGetVersionResponse = zServerVersionResponse */ export const zGetAccountResponse = zAccountResponse +export const zGetAccountSessionsQuery = z.object({ + limit: z.int().gte(1).lte(200).optional().default(100), + page: z.int().gte(1).optional().default(1), +}) + /** * Session list */ @@ -488,7 +532,7 @@ export const zGetAppsByAppIdDescribePath = z.object({ }) export const zGetAppsByAppIdDescribeQuery = z.object({ - fields: z.array(z.string()).optional(), + fields: z.string().optional(), workspace_id: z.string().optional(), }) @@ -526,7 +570,7 @@ export const zPostAppsByAppIdFormHumanInputByFormTokenPath = z.object({ /** * Form submitted */ -export const zPostAppsByAppIdFormHumanInputByFormTokenResponse = z.record(z.string(), z.unknown()) +export const zPostAppsByAppIdFormHumanInputByFormTokenResponse = zFormSubmitResponse export const zPostAppsByAppIdRunBody = zAppRunRequest @@ -557,7 +601,7 @@ export const zPostAppsByAppIdTasksByTaskIdStopPath = z.object({ /** * Task stopped */ -export const zPostAppsByAppIdTasksByTaskIdStopResponse = z.record(z.string(), z.unknown()) +export const zPostAppsByAppIdTasksByTaskIdStopResponse = zTaskStopResponse export const zPostOauthDeviceApproveBody = zDeviceMutateRequest diff --git a/packages/contracts/openapi-ts.api.config.ts b/packages/contracts/openapi-ts.api.config.ts index 122107eb78..445be5f06e 100644 --- a/packages/contracts/openapi-ts.api.config.ts +++ b/packages/contracts/openapi-ts.api.config.ts @@ -658,6 +658,14 @@ const isEmptySchemaObject = (value: unknown) => { return isObject(value) && Object.keys(value).length === 0 } +// A field the backend marked deliberately open via the `x-dify-opaque` vendor extension — e.g. a +// JSON Schema document or an app-config blob whose shape is genuinely arbitrary. Such a field is +// intentionally an open object, not an under-annotated one, so the readiness detector must not +// flag it (or its owning operation) as inaccurate. +const isIntentionallyOpaque = (schema: SwaggerSchema) => { + return (schema as { 'x-dify-opaque'?: unknown })['x-dify-opaque'] === true +} + const isLooseObjectSchema = (schema: SwaggerSchema) => { if (hasProperties(schema)) return false @@ -676,6 +684,9 @@ const hasLooseSchema = ( if (!schema) return true + if (isIntentionallyOpaque(schema)) + return false + const ref = schema?.$ref if (ref?.startsWith('#/definitions/')) { const refName = ref.slice('#/definitions/'.length) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d56d4da03e..338fd6b2e7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -641,6 +641,15 @@ importers: '@napi-rs/keyring': specifier: 'catalog:' version: 1.3.0 + '@orpc/client': + specifier: 'catalog:' + version: 1.14.5 + '@orpc/contract': + specifier: 'catalog:' + version: 1.14.5 + '@orpc/openapi-client': + specifier: 'catalog:' + version: 1.14.5 cli-table3: specifier: 'catalog:' version: 0.6.5