From 48a0d15d840c8f0485e334353efe4cda6345a168 Mon Sep 17 00:00:00 2001 From: Ricky Schema Cascade Date: Fri, 22 May 2026 23:25:53 +0200 Subject: [PATCH 1/2] fix(deploy): scope-aware + configKey-aware integration preflight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deploy CLI's preflight check ("is this provider connected?") produced both false positives and false negatives because the cloud surface it consulted was too coarse. - Default source on persona integrations is `deployer_user`, which the cloud resolves against `user_integrations`. The preflight only hit `workspace_integrations` → user-scoped connections always read as "not connected" (false negative) and orphan workspace rows from abandoned OAuth read as "connected" (false positive). - A workspace can have multiple Slack providers (slack-relay, slack-ricky, slack-nightcto, slack-my-senior-dev, slack-sage). A persona declaring `slack` (→ slack-relay) was previously satisfied by ANY provider:'slack' row regardless of which Nango config-key backed it. - `isConnectedStatus` accepted "any truthy connectionId" as connected, meaning a row left behind by an abandoned OAuth (with connectionId but no live session) tripped the check. Changes: - `IntegrationConnectResolver.isConnected` now takes `source` + `expectedConfigKey`. `relayfileIntegrationResolver` picks endpoint by source.kind: deployer_user → /me/integrations, workspace/* → /workspaces//integrations. Falls back to workspace endpoint with a warning if /me/integrations 404s (older cloud). - New `relayfileCatalogConfigKeyResolver` pulls /api/v1/integrations/catalog once per deploy, mapping provider id → Nango config-key. Walker passes the expected key through; rows whose providerConfigKey doesn't match are rejected. Missing field on the row falls back to provider-only match for compat with older cloud. - Tightened `isConnectedStatus`: dropped the connectionId-as-truth fallback. Only explicit status/state/ready/oauth.connected signals count. - Auto-wired catalog resolver for `mode: cloud` deploys; opt-in via `DeployResolvers.providerConfigKeys` otherwise. Tests: 112/112 pass, 8 new covering workspace vs me endpoint, configKey strict + fallback, orphan rejection, /me 404 fallback, catalog cache, catalog failure resilience. Depends on AgentWorkforce/cloud#988 for the cloud half (expose providerConfigKey on workspace listing + add /me/integrations). Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/deploy/src/connect.test.ts | 183 +++++++++++++++++++- packages/deploy/src/connect.ts | 251 +++++++++++++++++++++++++--- packages/deploy/src/deploy.test.ts | 15 +- packages/deploy/src/deploy.ts | 16 +- packages/deploy/src/index.ts | 2 + 5 files changed, 436 insertions(+), 31 deletions(-) diff --git a/packages/deploy/src/connect.test.ts b/packages/deploy/src/connect.test.ts index c7e0d5e3..857d9308 100644 --- a/packages/deploy/src/connect.test.ts +++ b/packages/deploy/src/connect.test.ts @@ -1,6 +1,10 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import { connectIntegrations, relayfileIntegrationResolver } from './connect.js'; +import { + connectIntegrations, + relayfileCatalogConfigKeyResolver, + relayfileIntegrationResolver +} from './connect.js'; import { createBufferedIO } from './io.js'; function okJson(body: unknown, status = 200): Response { @@ -10,7 +14,7 @@ function okJson(body: unknown, status = 200): Response { }); } -test('relayfileIntegrationResolver isConnected reads the cloud integration list', async () => { +test('relayfileIntegrationResolver isConnected defaults to /me/integrations (deployer_user)', async () => { const urls: string[] = []; const resolver = relayfileIntegrationResolver({ apiUrl: 'https://cloud.example.test', @@ -19,18 +23,182 @@ test('relayfileIntegrationResolver isConnected reads the cloud integration list' fetch: async (url) => { urls.push(String(url)); return okJson([ - { provider: 'github', status: 'ready', connectionId: 'conn-1' } + { provider: 'github', providerConfigKey: 'github-relay', status: 'ready' } ]); } }); assert.equal(await resolver.isConnected({ workspace: 'ws-runtime', provider: 'github' }), true); assert.equal(await resolver.isConnected({ workspace: 'ws-runtime', provider: 'notion' }), false); assert.deepEqual(urls, [ - 'https://cloud.example.test/api/v1/workspaces/ws-runtime/integrations', + 'https://cloud.example.test/api/v1/me/integrations', + 'https://cloud.example.test/api/v1/me/integrations' + ]); +}); + +test('relayfileIntegrationResolver isConnected hits /workspaces//integrations for workspace source', async () => { + const urls: string[] = []; + const resolver = relayfileIntegrationResolver({ + apiUrl: 'https://cloud.example.test', + workspaceId: 'ws-1', + workspaceToken: 'tok', + fetch: async (url) => { + urls.push(String(url)); + return okJson([ + { provider: 'github', providerConfigKey: 'github-relay', status: 'ready' } + ]); + } + }); + assert.equal( + await resolver.isConnected({ + workspace: 'ws-runtime', + provider: 'github', + source: { kind: 'workspace' } + }), + true + ); + assert.deepEqual(urls, [ 'https://cloud.example.test/api/v1/workspaces/ws-runtime/integrations' ]); }); +test('relayfileIntegrationResolver isConnected rejects rows whose providerConfigKey does not match', async () => { + // Workspace has slack-ricky connected. Persona declares plain `slack`, which + // should resolve to slack-relay. The row exists with provider:'slack' but + // backed by a different config-key → must NOT count as connected. + const resolver = relayfileIntegrationResolver({ + apiUrl: 'https://cloud.example.test', + workspaceId: 'ws-1', + workspaceToken: 'tok', + fetch: async () => + okJson([{ provider: 'slack', providerConfigKey: 'slack-ricky', status: 'ready' }]) + }); + assert.equal( + await resolver.isConnected({ + workspace: 'ws-1', + provider: 'slack', + expectedConfigKey: 'slack-relay' + }), + false + ); +}); + +test('relayfileIntegrationResolver isConnected accepts a matching providerConfigKey', async () => { + const resolver = relayfileIntegrationResolver({ + apiUrl: 'https://cloud.example.test', + workspaceId: 'ws-1', + workspaceToken: 'tok', + fetch: async () => + okJson([{ provider: 'slack', providerConfigKey: 'slack-relay', status: 'ready' }]) + }); + assert.equal( + await resolver.isConnected({ + workspace: 'ws-1', + provider: 'slack', + expectedConfigKey: 'slack-relay' + }), + true + ); +}); + +test('relayfileIntegrationResolver isConnected falls back to provider-name match when row lacks providerConfigKey', async () => { + // Older cloud (pre cloud#988) returns rows without providerConfigKey. To + // avoid hard-failing every deploy until that ships, the matcher treats a + // missing field as "trust the server" and matches by provider name only. + const resolver = relayfileIntegrationResolver({ + apiUrl: 'https://cloud.example.test', + workspaceId: 'ws-1', + workspaceToken: 'tok', + fetch: async () => okJson([{ provider: 'slack', status: 'ready' }]) + }); + assert.equal( + await resolver.isConnected({ + workspace: 'ws-1', + provider: 'slack', + expectedConfigKey: 'slack-relay' + }), + true + ); +}); + +test('relayfileIntegrationResolver isConnected ignores rows with only a connectionId (no status)', async () => { + // The previous matcher treated any truthy connectionId as connected. That + // caused false positives whenever an abandoned OAuth left an orphan row. + const resolver = relayfileIntegrationResolver({ + apiUrl: 'https://cloud.example.test', + workspaceId: 'ws-1', + workspaceToken: 'tok', + fetch: async () => okJson([{ provider: 'slack', connectionId: 'orphan' }]) + }); + assert.equal( + await resolver.isConnected({ workspace: 'ws-1', provider: 'slack' }), + false + ); +}); + +test('relayfileIntegrationResolver isConnected falls back to workspace endpoint when /me/integrations 404s', async () => { + const io = createBufferedIO(); + const urls: string[] = []; + const resolver = relayfileIntegrationResolver({ + apiUrl: 'https://cloud.example.test', + workspaceId: 'ws-1', + workspaceToken: 'tok', + io, + fetch: async (url) => { + urls.push(String(url)); + if (String(url).endsWith('/me/integrations')) { + return new Response('not found', { status: 404 }); + } + return okJson([{ provider: 'github', status: 'ready' }]); + } + }); + assert.equal( + await resolver.isConnected({ workspace: 'ws-runtime', provider: 'github' }), + true + ); + assert.deepEqual(urls, [ + 'https://cloud.example.test/api/v1/me/integrations', + 'https://cloud.example.test/api/v1/workspaces/ws-runtime/integrations' + ]); + assert.ok( + io.messages.some( + (m) => m.level === 'warn' && /me\/integrations/.test(m.message) + ) + ); +}); + +test('relayfileCatalogConfigKeyResolver returns the expected configKey and caches the catalog', async () => { + let calls = 0; + const resolver = relayfileCatalogConfigKeyResolver({ + apiUrl: 'https://cloud.example.test', + workspaceToken: 'tok', + fetch: async () => { + calls += 1; + return okJson({ + providers: [ + { id: 'slack', configKey: 'slack-relay' }, + { id: 'github', configKey: 'github-relay' } + ] + }); + } + }); + assert.equal(await resolver.resolve('slack'), 'slack-relay'); + assert.equal(await resolver.resolve('github'), 'github-relay'); + assert.equal(await resolver.resolve('unknown'), undefined); + assert.equal(calls, 1, 'catalog should be fetched exactly once and cached'); +}); + +test('relayfileCatalogConfigKeyResolver returns undefined for every provider when catalog fetch fails', async () => { + const io = createBufferedIO(); + const resolver = relayfileCatalogConfigKeyResolver({ + apiUrl: 'https://cloud.example.test', + workspaceToken: 'tok', + io, + fetch: async () => new Response('boom', { status: 500 }) + }); + assert.equal(await resolver.resolve('slack'), undefined); + assert.ok(io.messages.some((m) => m.level === 'warn' && /catalog/.test(m.message))); +}); + test('relayfileIntegrationResolver reads the latest workspace token for each request', async () => { let token = 'old-token'; const authHeaders: string[] = []; @@ -39,12 +207,13 @@ test('relayfileIntegrationResolver reads the latest workspace token for each req workspaceId: 'ws-1', workspaceToken: () => token, fetch: async (_url, init) => { - authHeaders.push(String(new Headers(init?.headers).get('authorization'))); - if (authHeaders.length === 1) { + const auth = String(new Headers(init?.headers).get('authorization')); + authHeaders.push(auth); + if (auth === 'Bearer old-token') { return okJson({ error: 'Unauthorized' }, 401); } return okJson([ - { provider: 'github', status: 'ready', connectionId: 'conn-1' } + { provider: 'github', providerConfigKey: 'github-relay', status: 'ready' } ]); } }); diff --git a/packages/deploy/src/connect.ts b/packages/deploy/src/connect.ts index 2ce86a52..7046b757 100644 --- a/packages/deploy/src/connect.ts +++ b/packages/deploy/src/connect.ts @@ -1,6 +1,6 @@ import { platform } from 'node:os'; import { spawn } from 'node:child_process'; -import type { PersonaSpec } from '@agentworkforce/persona-kit'; +import type { IntegrationSource, PersonaSpec } from '@agentworkforce/persona-kit'; import type { DeployIO, IntegrationConnectOutcome } from './types.js'; /** @@ -30,8 +30,29 @@ const PROVIDER_ENV_PREFIX = 'WORKFORCE_INTEGRATION_'; * dep tree (smaller bin, faster install). */ export interface IntegrationConnectResolver { - /** Is the provider already linked to the workspace? */ - isConnected(args: { workspace: string; provider: string }): Promise; + /** + * Is the provider already linked to the right scope for this persona? + * + * `source` discriminates which scope the cloud will resolve at dispatch + * time (deployer-user / workspace / workspace-service-account). The + * preflight check must hit the same scope: a `user_integrations` row + * does not satisfy a workspace-scoped persona declaration and vice + * versa. Defaults to `{ kind: 'deployer_user' }` (matches the + * persona-kit default at parse time). + * + * `expectedConfigKey` is the Nango provider-config-key the persona's + * declared `provider` resolves to (e.g. provider `slack` → + * `slack-relay`). When supplied, rows whose `providerConfigKey` does + * not match are ignored — protecting against false positives when the + * workspace has multiple Slack providers (slack-relay / slack-ricky / + * slack-nightcto / slack-my-senior-dev / slack-sage). + */ + isConnected(args: { + workspace: string; + provider: string; + source?: IntegrationSource; + expectedConfigKey?: string; + }): Promise; /** Run the browser-based OAuth flow and resolve when the user finishes. */ connect(args: { workspace: string; provider: string }): Promise<{ connectionId: string }>; } @@ -95,13 +116,25 @@ export function relayfileIntegrationResolver(opts: { const apiUrl = opts.apiUrl.replace(/\/+$/, ''); return { - async isConnected({ workspace, provider }) { + async isConnected({ workspace, provider, source, expectedConfigKey }) { const workspaceId = workspace || opts.workspaceId; const token = await resolveWorkspaceToken(opts.workspaceToken); - const body = await requestJson(fetchImpl, `${apiUrl}/api/v1/workspaces/${encodeURIComponent( - workspaceId - )}/integrations`, token); - return listHasConnectedProvider(body, provider); + const effectiveSource: IntegrationSource = source ?? { kind: 'deployer_user' }; + + const list = await fetchIntegrationsForScope({ + fetchImpl, + apiUrl, + token, + workspaceId, + source: effectiveSource, + io + }); + return listHasConnectedProvider(list, provider, { + ...(expectedConfigKey ? { expectedConfigKey } : {}), + ...(effectiveSource.kind === 'workspace_service_account' + ? { serviceAccountName: effectiveSource.name } + : {}) + }); }, async connect({ workspace, provider }) { const workspaceId = workspace || opts.workspaceId; @@ -174,6 +207,28 @@ export interface ConnectAllInput { authRecovery?: IntegrationAuthRecoveryResolver; /** Required only when persona.useSubscription is true. */ subscription?: ProviderSubscriptionResolver; + /** + * Optional resolver for the Nango provider-config-key behind each provider + * id. Backed by `GET /api/v1/integrations/catalog`. When supplied, the + * walker passes the expected config-key into the resolver's `isConnected` + * call so e.g. a persona declaring `slack` is verified against the + * `slack-relay` config-key specifically, ignoring rows backed by other + * Slack providers (slack-ricky / slack-nightcto / slack-sage / etc.). + * + * When omitted, the walker falls back to provider-name-only matching, + * which is sufficient for providers that have a single config-key but + * loses precision for ambiguous ones. + */ + providerConfigKeys?: ProviderConfigKeyResolver; +} + +/** + * Returns the expected Nango provider-config-key for a persona-declared + * provider id. The CLI implementation caches the cloud catalog response + * after the first lookup to keep deploys cheap. + */ +export interface ProviderConfigKeyResolver { + resolve(provider: string): Promise; } export interface ConnectAllResult { @@ -204,10 +259,22 @@ export async function connectIntegrations(input: ConnectAllInput): Promise undefined) + : undefined; + let statusCheckFailure: string | undefined; - let connected = await checkProviderConnected(input, provider, (message) => { - statusCheckFailure = message; - }); + let connected = await checkProviderConnected( + input, + provider, + source, + expectedConfigKey, + (message) => { + statusCheckFailure = message; + } + ); if (connected) { input.io.info(`integrations.${provider}: already connected`); @@ -232,9 +299,15 @@ export async function connectIntegrations(input: ConnectAllInput): Promise { - statusCheckFailure = message; - }); + connected = await checkProviderConnected( + input, + provider, + source, + expectedConfigKey, + (message) => { + statusCheckFailure = message; + } + ); if (connected) { input.io.info(`integrations.${provider}: already connected`); outcomes.push({ provider, status: 'already-connected' }); @@ -347,10 +420,17 @@ export async function connectIntegrations(input: ConnectAllInput): Promise void ): Promise { return await input.integrations - .isConnected({ workspace: input.workspace, provider }) + .isConnected({ + workspace: input.workspace, + provider, + source, + ...(expectedConfigKey ? { expectedConfigKey } : {}) + }) .catch((err) => { const message = err instanceof Error ? err.message : String(err); onFailure(message); @@ -401,7 +481,67 @@ async function resolveWorkspaceToken(token: string | (() => string | Promise/integrations` + * (reads `workspace_integrations`). + * + * Backwards-compat: if `/me/integrations` is unavailable (older cloud that + * hasn't shipped cloud#988 yet), fall back to the workspace endpoint with a + * warning. Authors deploying personas with `source: { kind: 'deployer_user' }` + * against an older cloud will see false negatives, but the deploy still + * surfaces a clean error rather than silently mis-resolving. + */ +async function fetchIntegrationsForScope(args: { + fetchImpl: typeof fetch; + apiUrl: string; + token: string; + workspaceId: string; + source: IntegrationSource; + io?: Pick; +}): Promise { + if (args.source.kind === 'deployer_user') { + const url = `${args.apiUrl}/api/v1/me/integrations`; + try { + return await requestJson(args.fetchImpl, url, args.token); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (/\b404\b/.test(message) || /\b405\b/.test(message)) { + args.io?.warn?.( + 'cloud does not expose /api/v1/me/integrations yet; falling back to the workspace integrations list. ' + + 'Deployer-user-scoped connections may show as not-connected. ' + + 'Tracking in AgentWorkforce/cloud#988.' + ); + return await requestJson( + args.fetchImpl, + `${args.apiUrl}/api/v1/workspaces/${encodeURIComponent(args.workspaceId)}/integrations`, + args.token + ); + } + throw err; + } + } + return await requestJson( + args.fetchImpl, + `${args.apiUrl}/api/v1/workspaces/${encodeURIComponent(args.workspaceId)}/integrations`, + args.token + ); +} + +interface MatchOpts { + expectedConfigKey?: string; + serviceAccountName?: string; +} + +function listHasConnectedProvider( + body: unknown, + provider: string, + opts: MatchOpts = {} +): boolean { const candidates = Array.isArray(body) ? body : body && typeof body === 'object' && Array.isArray((body as { integrations?: unknown }).integrations) @@ -410,10 +550,34 @@ function listHasConnectedProvider(body: unknown, provider: string): boolean { return candidates.some((item) => { if (!item || typeof item !== 'object' || Array.isArray(item)) return false; const record = item as Record; - return record.provider === provider && isConnectedStatus(record); + if (record.provider !== provider) return false; + if (opts.expectedConfigKey) { + const rowConfigKey = readString(record, 'providerConfigKey'); + // If the row carries a providerConfigKey, enforce strict match. + // If the field is missing entirely (older cloud that hasn't shipped + // cloud#988), fall through to status-only matching — the cloud + // server will still resolve the right config-key at dispatch time. + if (rowConfigKey !== undefined && rowConfigKey !== opts.expectedConfigKey) { + return false; + } + } + if (opts.serviceAccountName) { + const rowName = readString(record, 'name') ?? readString(record, 'serviceAccountName'); + if (rowName !== opts.serviceAccountName) return false; + } + return isConnectedStatus(record); }); } +/** + * A row counts as "connected" only when the cloud's derived state is + * affirmatively healthy. The previous implementation also accepted "any + * truthy connectionId" as a yes, which produced false positives whenever a + * stale row was left behind by an abandoned OAuth attempt. The cloud now + * derives `status` from `initialSync + writeback` (see + * `cloud/packages/web/app/api/v1/workspaces/[workspaceId]/integrations/route.ts:62`), + * so trusting that field is both correct and sufficient. + */ function isConnectedStatus(value: unknown): boolean { if (!value || typeof value !== 'object' || Array.isArray(value)) return false; const record = value as Record; @@ -423,13 +587,62 @@ function isConnectedStatus(value: unknown): boolean { || record.state === 'connected' || record.state === 'ready' || record.ready === true - || Boolean(record.connectionId) - || Boolean(record.currentConnectionId) || (record.oauth !== null && typeof record.oauth === 'object' && (record.oauth as { connected?: unknown }).connected === true); } +/** + * Resolver backed by `GET /api/v1/integrations/catalog`. The catalog is + * pulled once on first lookup and cached for the lifetime of this + * instance. Designed to be plugged into `ConnectAllInput.providerConfigKeys` + * so the walker can pass a `expectedConfigKey` into `isConnected` calls. + */ +export function relayfileCatalogConfigKeyResolver(opts: { + apiUrl: string; + workspaceToken: string | (() => string | Promise); + fetch?: typeof fetch; + io?: Pick; +}): ProviderConfigKeyResolver { + const fetchImpl = opts.fetch ?? fetch; + const apiUrl = opts.apiUrl.replace(/\/+$/, ''); + let cache: Promise> | null = null; + + const load = async (): Promise> => { + const token = await resolveWorkspaceToken(opts.workspaceToken); + const body = await requestJson(fetchImpl, `${apiUrl}/api/v1/integrations/catalog`, token); + const entries = Array.isArray(body) + ? body + : body && typeof body === 'object' && Array.isArray((body as { providers?: unknown }).providers) + ? (body as { providers: unknown[] }).providers + : []; + const map = new Map(); + for (const entry of entries) { + const id = readString(entry, 'id'); + const configKey = readString(entry, 'configKey') ?? readString(entry, 'defaultConfigKey'); + if (id && configKey) map.set(id, configKey); + } + return map; + }; + + return { + async resolve(provider) { + if (!cache) { + cache = load().catch((err) => { + const message = err instanceof Error ? err.message : String(err); + opts.io?.warn?.( + `cloud integration catalog fetch failed (${message}); falling back to provider-name-only matching for this deploy` + ); + cache = null; + return new Map(); + }); + } + const map = await cache; + return map.get(provider); + } + }; +} + function readString(value: unknown, field: string): string | undefined { if (!value || typeof value !== 'object' || Array.isArray(value)) return undefined; const raw = (value as Record)[field]; diff --git a/packages/deploy/src/deploy.test.ts b/packages/deploy/src/deploy.test.ts index 0854800f..f2fb7f89 100644 --- a/packages/deploy/src/deploy.test.ts +++ b/packages/deploy/src/deploy.test.ts @@ -398,13 +398,20 @@ test('deploy can recover cloud integration auth by logging in and retrying with return { token: 'fresh-token' }; } }; - globalThis.fetch = (async (_input, init) => { - authHeaders.push(String(new Headers(init?.headers).get('authorization'))); - if (authHeaders.length === 1) { + globalThis.fetch = (async (input, init) => { + const url = String(input); + const auth = String(new Headers(init?.headers).get('authorization')); + // Catalog fetch is best-effort: don't drive auth recovery from it. + // Return an empty providers list so the resolver caches that and moves on. + if (url.includes('/api/v1/integrations/catalog')) { + return jsonResponse({ providers: [] }); + } + authHeaders.push(auth); + if (auth === 'Bearer stale-token') { return jsonResponse({ error: 'Unauthorized' }, 401); } return jsonResponse([ - { provider: 'notion', status: 'ready', connectionId: 'conn-notion' } + { provider: 'notion', providerConfigKey: 'notion-relay', status: 'ready' } ]); }) as typeof fetch; diff --git a/packages/deploy/src/deploy.ts b/packages/deploy/src/deploy.ts index 3211860a..532b6641 100644 --- a/packages/deploy/src/deploy.ts +++ b/packages/deploy/src/deploy.ts @@ -6,10 +6,12 @@ import { resolveCloudUrl } from './cloud-url.js'; import { connectIntegrations, envIntegrationResolver, + relayfileCatalogConfigKeyResolver, relayfileIntegrationResolver, type ConnectAllInput, type IntegrationAuthRecoveryResolver, type IntegrationConnectResolver, + type ProviderConfigKeyResolver, type ProviderSubscriptionResolver } from './connect.js'; import { createTerminalIO } from './io.js'; @@ -44,6 +46,7 @@ export interface DeployResolvers { integrations?: IntegrationConnectResolver; authRecovery?: CloudAuthRecoveryResolver; subscription?: ProviderSubscriptionResolver; + providerConfigKeys?: ProviderConfigKeyResolver; bundle?: BundleStager; modes?: Partial>; } @@ -206,7 +209,18 @@ export async function deploy(opts: DeployOptions, resolvers: DeployResolvers = { ) } : {}), - ...(resolvers.subscription ? { subscription: resolvers.subscription } : {}) + ...(resolvers.subscription ? { subscription: resolvers.subscription } : {}), + ...(resolvers.providerConfigKeys + ? { providerConfigKeys: resolvers.providerConfigKeys } + : mode === 'cloud' + ? { + providerConfigKeys: relayfileCatalogConfigKeyResolver({ + apiUrl: normalizeCloudUrl(cloudUrl ?? defaultApiUrl()), + workspaceToken: () => activeToken, + io + }) + } + : {}) }); const bundleDir = path.resolve( diff --git a/packages/deploy/src/index.ts b/packages/deploy/src/index.ts index e434adea..2c0e3739 100644 --- a/packages/deploy/src/index.ts +++ b/packages/deploy/src/index.ts @@ -20,11 +20,13 @@ export { preflightPersona }; export { connectIntegrations, envIntegrationResolver, + relayfileCatalogConfigKeyResolver, relayfileIntegrationResolver, type ConnectAllInput, type ConnectAllResult, type IntegrationAuthRecoveryResolver, type IntegrationConnectResolver, + type ProviderConfigKeyResolver, type ProviderSubscriptionResolver } from './connect.js'; export { From 652e93d500bae748d615dadc3bc31dbd33a7865d Mon Sep 17 00:00:00 2001 From: Ricky Schema Cascade Date: Fri, 22 May 2026 23:59:18 +0200 Subject: [PATCH 2/2] fix(deploy): check explicit HTTP status for /me/integrations fallback Previously fetchIntegrationsForScope regex-matched "404"/"405" anywhere in the error message to decide whether to fall back to the workspace endpoint. A 5xx response whose body happened to contain "404" (e.g. upstream timeout citing a request id like "404abc", proxy error mentioning an absent route) would wrongly suppress the real failure and silently route through the workspace listing. Tag CloudRequestError with the numeric HTTP status and branch on err.status === 404 || err.status === 405 instead. Other failures (auth, 5xx, network) propagate untouched so the existing auth-recovery and clean-error paths still fire. Regression test added: 500 with "404" in the body must not trigger fallback. Addresses PR #131 review feedback from cubic + CodeRabbit. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/deploy/src/connect.test.ts | 23 ++++++++++++++++ packages/deploy/src/connect.ts | 42 ++++++++++++++++++++++++----- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/packages/deploy/src/connect.test.ts b/packages/deploy/src/connect.test.ts index 857d9308..d1017add 100644 --- a/packages/deploy/src/connect.test.ts +++ b/packages/deploy/src/connect.test.ts @@ -135,6 +135,29 @@ test('relayfileIntegrationResolver isConnected ignores rows with only a connecti ); }); +test('relayfileIntegrationResolver isConnected does NOT fall back when /me/integrations returns 5xx with "404" in the body', async () => { + // Regression: previous implementation regex-matched "404" anywhere in the + // error message and treated a 500 whose body mentioned "/api/v1/foo/404" + // (or any other 404 substring) as a missing-endpoint signal. The check + // must use the explicit HTTP status, not the body text. + const urls: string[] = []; + const resolver = relayfileIntegrationResolver({ + apiUrl: 'https://cloud.example.test', + workspaceId: 'ws-1', + workspaceToken: 'tok', + fetch: async (url) => { + urls.push(String(url)); + return new Response('upstream timeout (request 404abc failed)', { status: 500 }); + } + }); + await assert.rejects( + resolver.isConnected({ workspace: 'ws-runtime', provider: 'github' }), + /cloud integration request failed: 500/ + ); + // Only the /me call should have happened — no silent fallback to workspace. + assert.deepEqual(urls, ['https://cloud.example.test/api/v1/me/integrations']); +}); + test('relayfileIntegrationResolver isConnected falls back to workspace endpoint when /me/integrations 404s', async () => { const io = createBufferedIO(); const urls: string[] = []; diff --git a/packages/deploy/src/connect.ts b/packages/deploy/src/connect.ts index 7046b757..61978c41 100644 --- a/packages/deploy/src/connect.ts +++ b/packages/deploy/src/connect.ts @@ -439,6 +439,26 @@ async function checkProviderConnected( }); } +/** + * Error thrown by `requestJson` for any non-2xx response. Carries the numeric + * HTTP `status` so callers can branch on it without parsing the message + * (which can include the response body — body content like "404" inside a + * 500 response was causing false-positive fallbacks). + */ +interface CloudRequestError extends Error { + status: number; +} + +function cloudRequestError(message: string, status: number): CloudRequestError { + const err = new Error(message) as CloudRequestError; + err.status = status; + return err; +} + +function isCloudRequestError(err: unknown): err is CloudRequestError { + return err instanceof Error && typeof (err as { status?: unknown }).status === 'number'; +} + async function requestJson( fetchImpl: typeof fetch, url: string, @@ -454,17 +474,22 @@ async function requestJson( } }); if (res.status === 401) { - throw new Error( - 'cloud integration request failed: unauthorized. Your active workspace session is invalid or expired. Run `agentworkforce login --workspace ` to refresh, then retry.' + throw cloudRequestError( + 'cloud integration request failed: unauthorized. Your active workspace session is invalid or expired. Run `agentworkforce login --workspace ` to refresh, then retry.', + 401 ); } if (res.status === 403) { - throw new Error( - 'cloud integration request failed: forbidden. The active account is not authorized for this workspace. Run `agentworkforce login --workspace ` against an account with access, then retry.' + throw cloudRequestError( + 'cloud integration request failed: forbidden. The active account is not authorized for this workspace. Run `agentworkforce login --workspace ` against an account with access, then retry.', + 403 ); } if (!res.ok) { - throw new Error(`cloud integration request failed: ${res.status} ${await res.text().catch(() => '')}`.trim()); + throw cloudRequestError( + `cloud integration request failed: ${res.status} ${await res.text().catch(() => '')}`.trim(), + res.status + ); } return await res.json(); } @@ -509,8 +534,11 @@ async function fetchIntegrationsForScope(args: { try { return await requestJson(args.fetchImpl, url, args.token); } catch (err) { - const message = err instanceof Error ? err.message : String(err); - if (/\b404\b/.test(message) || /\b405\b/.test(message)) { + // Only fall back when the endpoint itself is missing (older cloud that + // hasn't shipped cloud#988). Any other failure — auth, 5xx, network — + // must propagate so callers see the real error and can drive the + // existing auth-recovery flow rather than silently masking the cause. + if (isCloudRequestError(err) && (err.status === 404 || err.status === 405)) { args.io?.warn?.( 'cloud does not expose /api/v1/me/integrations yet; falling back to the workspace integrations list. ' + 'Deployer-user-scoped connections may show as not-connected. ' +