feat(workflow-executor): use stored OAuth credentials for MCP steps [PRD-367]

[hercemer42]

What & why

Executor-side runtime use of stored OAuth credentials for OAuth2-protected MCP servers (PRD-367, PR2). At an oauth2 MCP step the executor looks up the stored credential by (user, server), runs a refresh-token grant against the stored token_endpoint behind an expiry-skew cache, injects the Bearer token before connecting, retries once on an upstream 401 (covering both list-tools and the tool call), and pauses the step awaiting-input with awaitingInputReason: needs-oauth-reauth when there is no usable credential or the refresh is rejected. Bearer/none steps are unchanged.

Stacked PR / deploy ordering

• Based on feat/prd-367-pr1-executor-oauth-credentials (PR feat(workflow-executor): add OAuth credential store + deposit endpoint (PRD-367 PR1) #1619), not main: PR2 depends on PR1's credential store / encryption / deposit endpoint, which is not yet merged. Review the diff against that branch.
• Behaviour stays dormant until the Forest server serves authType + accepts the typed awaitingInputReason (PR1.5) and the frontend ships (PR3) — deploy the orchestrator first. Safe to deploy alone: the oauth2 path only activates once authType=oauth2 is served; bearer/none is unchanged.

Notable choices (large-PR annotation)

• Serialization (option B): in-process mutex + DB re-read + single retry on invalid_grant, not a Postgres row lock — the executor and token endpoints can sit behind a client VPN, so no DB lock is held across the refresh HTTP call. Row lock is the documented hardening path if a strict reuse-detection provider plus real cross-process contention appears.
• Executor isolation: the executor gains only a re-loadable tool source (reloadWithFreshAuth) plus a typed OAuthReauthRequiredError -> awaiting-input mapping; all token/credential/HTTP logic stays behind RemoteToolFetcher and the new OAuth token service.
• Shared @forestadmin/ai-proxy change (consumed by the agent too) is additive-only (new loadToolsWithFailures / loadRemoteToolsWithFailures + mcp-auth-error export) plus behaviour-preserving delegation; authType is stripped in the McpClient constructor like id.
• In-memory executor (dev-only) raises a ConfigurationError for oauth2 steps (no credential store wired).

Tests

294 tests across the touched suites (ai-proxy 59 + workflow-executor 235); build, lint, tsc clean; >=90% line/stmt coverage on changed files.

Known limitation

A re-auth at tool-call time leaves the step at idempotencyPhase=executing, so it needs a manual retry after re-auth; the common list-tools-time re-auth (pre-executor) resumes cleanly.

Part of PRD-367.

🤖 Generated with Claude Code

Note

Add stored OAuth credential support for MCP steps in workflow executor

• Introduces OAuthTokenService to retrieve, cache, and refresh OAuth access tokens per (userId, mcpServerId), with per-key serialization via a new KeyedMutex and rotation-aware retry logic.
• Adds refreshAccessToken and assertSafeTokenEndpoint to perform the OAuth2 refresh-token grant and validate token endpoints against SSRF risks before any network call.
• Updates RemoteToolFetcher.fetch to inject Bearer tokens for OAuth2 MCP servers, detect auth failures, and retry once with a forced refresh before surfacing OAuthReauthRequiredError.
• Updates McpStepExecutor and StepExecutorFactory so auth failures during tool invocation produce an awaiting-input outcome with reason needs-oauth-reauth instead of failing the step.
• Adds a GET /list-mcp-tools HTTP endpoint for design-time tool listing, with typed 409/503 responses for reauth and missing encryption key cases.
• POST /mcp-oauth-credentials now validates tokenEndpoint safety at deposit time; DELETE now evicts cached tokens immediately.
• Risk: Runner and ExecutorHttpServer constructor signatures have changed to require OAuth-related dependencies.

^{Macroscope summarized 81b5607.}

[linear-code]

PRD-367

PRD-624

[qltysh]

12 new issues

Tool	Category	Rule	Count
qlty	Structure	Function with many returns (count = 9): create	5
qlty	Structure	Function with many parameters (count = 4): constructor	4
qlty	Structure	Function with high complexity (count = 12): invokeWithReauthRetry	3

[qltysh]

Coverage Impact

⬆️ Merging this pull request will increase total coverage on feat/prd-367-pr1-executor-oauth-credentials by 0.04%.

Modified Files with Diff Coverage (22)

Rating	File	% Diff	Uncovered Line #s
	packages/workflow-executor/src/runner.ts	100.0%
	packages/workflow-executor/src/executors/step-executor-factory.ts	100.0%
	packages/workflow-executor/src/build-workflow-executor.ts	100.0%
	packages/workflow-executor/src/remote-tool-fetcher.ts	100.0%
	packages/workflow-executor/src/http/executor-http-server.ts	95.5%	348
	packages/workflow-executor/src/adapters/server-ai-adapter.ts	100.0%
	packages/workflow-executor/src/types/validated/step-outcome.ts	100.0%
	packages/ai-proxy/src/mcp-client.ts	100.0%
	packages/workflow-executor/src/adapters/ai-client-adapter.ts	100.0%
	packages/ai-proxy/src/ai-client.ts	100.0%
	packages/workflow-executor/src/errors.ts	100.0%
	packages/workflow-executor/src/executors/mcp-step-executor.ts	100.0%
	...ow-executor/src/adapters/step-outcome-to-update-step-mapper.ts	100.0%
	...s/workflow-executor/src/adapters/always-error-ai-model-port.ts	100.0%
	packages/workflow-executor/src/defaults.ts	100.0%
	packages/ai-proxy/src/index.ts	100.0%
	packages/workflow-executor/src/http/mcp-oauth-credentials.ts	100.0%
	packages/workflow-executor/src/oauth/refresh-grant.ts	100.0%
	packages/workflow-executor/src/oauth/token-service.ts	98.2%	153
	packages/workflow-executor/src/oauth/token-endpoint-url.ts	100.0%
	packages/ai-proxy/src/mcp-auth-error.ts	92.3%	25-27
	packages/workflow-executor/src/oauth/keyed-mutex.ts	100.0%
	Total	98.6%

🤖 Increase coverage with AI coding...

In the `feat/prd-367-pr2-executor-oauth-runtime` branch, add test coverage for this new code:

- `packages/ai-proxy/src/mcp-auth-error.ts` -- Line 25-27
- `packages/workflow-executor/src/http/executor-http-server.ts` -- Line 348
- `packages/workflow-executor/src/oauth/token-service.ts` -- Line 153

🚦 See full report on Qlty Cloud »

🛟 Help

• Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

• Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

• File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

  • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

[hercemer42]

Agent code review — Claude Opus 4.8 (2026-06-18)

Reviewed the folded-in commits 7bf9357b (key-rotation re-consent) and 842d4680 (GET /list-mcp-tools) via the pr-review-toolkit code-reviewer; the runtime/max-run path was covered in the earlier review pass. No issues found.

Verified:

• list-mcp-tools — user_id is taken from the validated JWT (never the request); the mcpServerId query is validated (400 if absent). Responses leak no secrets (200 → {name, description} per tool; 409 → {awaitingInputReason, mcpServerId}). The typed needs-oauth-reauth 409 is set on ctx directly so it survives the error middleware (which would otherwise remap OAuthReauthRequiredError → 400).
• Key-rotation classification (toGrantParams) — the try wraps only the decrypt, so a missing key stays terminal (ExecutorEncryptionKeyMissingError rethrown) and a key-present decrypt failure → needs-oauth-reauth; the grant call sits outside the try, so invalid_grant/transient/5xx errors cannot be misclassified into a re-consent loop.
• Domain error classes (no raw Error), no PRD refs in comments, tool serialization type-safe.

[Scra3]

Review of the OAuth2 MCP executor runtime (PRD-367 PR2). One blocking security issue (SSRF via redirect on the token endpoint) plus a few non-blocking hardening points. Note: the tool-call re-auth idempotency-phase issue is already fixed in the stacked #1724, so it's not raised here.

[Scra3]

issue (blocking): fetch() follows redirects by default, so a deposited token endpoint that passes assertSafeTokenEndpoint but 3xx-redirects to 169.254.169.254/loopback re-sends the POST body to the internal host — leaking the refresh_token (and the client_secret in client_secret_post mode) and parsing the internal response as a token; pass redirect: 'manual' and treat any 3xx as an OAuthRefreshError.

[Scra3]

issue (non-blocking): the POST deposit handler has no symmetric evict(), so re-depositing a credential (reconnect without a prior DELETE) keeps serving the previously cached access token until it expires — mirror this evict on the deposit path.

[Scra3]

issue (non-blocking): evict() runs outside the KeyedMutex, so a disconnect landing during an in-flight refresh no-ops on the empty cache and this cache.set then re-populates a just-deleted credential (served until expiry); it is also process-local, so on a multi-instance deployment other instances never drop the token — consider an eviction epoch checked before cache.set.

[Scra3]

suggestion: the loopback block and the https requirement are gated on NODE_ENV === 'production', so a real deployment with NODE_ENV unset or misspelled silently allows loopback targets and cleartext http — prefer failing closed and opting into the relaxation for dev/test explicitly.