From af46fdbb96c25ae9cec22e3b2c713823b675917e Mon Sep 17 00:00:00 2001 From: Ricky Schema Cascade Date: Fri, 4 Sep 2026 14:58:23 +0200 Subject: [PATCH 1/2] fix(deploy): preserve implicit integration source intent Legacy personas omit source and rely on workspace fallback. Compiled JSON discarded the private marker, making CI deploys reject healthy workspace connections and send operators toward unnecessary reconnects. Keep omission as the durable public JSON representation while retaining the in-memory default, and report stored source or provider-config mismatches separately from actual connection absence. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BmMcdDakbnDkHBxG4pit7z --- packages/deploy/src/compile-agent.test.ts | 25 +++ packages/deploy/src/connect.test.ts | 262 +++++++++++++++++++++- packages/deploy/src/connect.ts | 235 +++++++++++++++++-- packages/deploy/src/deploy.ts | 14 +- packages/persona-kit/src/index.ts | 1 + packages/persona-kit/src/parse.test.ts | 20 ++ packages/persona-kit/src/parse.ts | 40 +++- 7 files changed, 568 insertions(+), 29 deletions(-) diff --git a/packages/deploy/src/compile-agent.test.ts b/packages/deploy/src/compile-agent.test.ts index d236e809..1b261482 100644 --- a/packages/deploy/src/compile-agent.test.ts +++ b/packages/deploy/src/compile-agent.test.ts @@ -88,6 +88,31 @@ test('preflight evaluates a single-file Agent source exactly once', async () => } }); +test('compiled JSON preserves implicit source as omission without leaking parser metadata', async () => { + const dir = await mkdtemp(path.join(tmpdir(), 'compiled-agent-source-roundtrip-')); + try { + const implicitPath = path.join(dir, 'implicit-agent.ts'); + const explicitPath = path.join(dir, 'explicit-agent.ts'); + await writeFile(implicitPath, PRESET); + await writeFile( + explicitPath, + PRESET.replace( + 'integrations: { github: {} },', + "integrations: { github: { source: { kind: 'deployer_user' } } }," + ) + ); + + const implicit = projectCompiledAgentForPersistence(await compileAgentSource(implicitPath)); + const explicit = projectCompiledAgentForPersistence(await compileAgentSource(explicitPath)); + + assert.equal(implicit.persona.integrations?.github.source, undefined); + assert.equal(JSON.stringify(implicit).includes('__agentworkforceImplicitSource'), false); + assert.deepEqual(explicit.persona.integrations?.github.source, { kind: 'deployer_user' }); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + test('single-file detection routes invalid persona fields to precise validation', async () => { const dir = await mkdtemp(path.join(tmpdir(), 'compiled-agent-invalid-')); try { diff --git a/packages/deploy/src/connect.test.ts b/packages/deploy/src/connect.test.ts index e561b7ae..c6053be2 100644 --- a/packages/deploy/src/connect.test.ts +++ b/packages/deploy/src/connect.test.ts @@ -1,6 +1,6 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import type { PersonaSpec } from '@agentworkforce/persona-kit'; +import { parsePersonaSpec, type PersonaSpec } from '@agentworkforce/persona-kit'; import { collectPickerInputs, connectIntegrations, @@ -315,6 +315,266 @@ test('relayfileIntegrationResolver isConnected does not widen explicit deployer_ 'https://cloud.example.test/api/v1/workspaces/ws-runtime/integrations/slack/status?scope=deployer_user' ]); }); + +test('relayfileIntegrationResolver lists stored connection sources without conflating owners', async () => { + const urls: string[] = []; + const resolver = relayfileIntegrationResolver({ + apiUrl: 'https://cloud.example.test', + workspaceId: 'ws-1', + workspaceToken: 'tok', + fetch: async (url) => { + urls.push(String(url)); + if (String(url).endsWith('/api/v1/me/integrations')) { + return okJson({ + integrations: [ + { + provider: 'github', + providerConfigKey: 'github-user', + connectionId: 'conn-user' + } + ] + }); + } + return okJson({ + integrations: [ + { + provider: 'github', + providerConfigKey: 'github-relay', + connectionId: 'conn-workspace', + scope: 'workspace' + }, + { + provider: 'github', + providerConfigKey: 'github-relay', + connectionId: 'conn-service', + scope: 'workspace_service_account', + serviceAccountName: 'release-bot' + }, + { provider: 'slack', connectionId: 'conn-other' } + ] + }); + } + }); + + assert.ok(resolver.listConnectionSources); + assert.deepEqual( + await resolver.listConnectionSources({ workspace: 'ws-runtime', provider: 'github' }), + [ + { source: { kind: 'deployer_user' }, providerConfigKey: 'github-user' }, + { source: { kind: 'workspace' }, providerConfigKey: 'github-relay' }, + { + source: { kind: 'workspace_service_account', name: 'release-bot' }, + providerConfigKey: 'github-relay' + } + ] + ); + assert.deepEqual(urls, [ + 'https://cloud.example.test/api/v1/me/integrations', + 'https://cloud.example.test/api/v1/workspaces/ws-runtime/integrations' + ]); +}); + +test('relayfileIntegrationResolver can diagnose workspace sources with a CI workspace token', async () => { + const resolver = relayfileIntegrationResolver({ + apiUrl: 'https://cloud.example.test', + workspaceId: 'ws-1', + workspaceToken: 'workspace-token', + fetch: async (url) => { + if (String(url).endsWith('/api/v1/me/integrations')) { + return okJson({ error: 'workspace tokens have no user' }, 401); + } + return okJson({ + integrations: [ + { + provider: 'github', + provider_config_key: 'github-relay', + connectionId: 'conn-workspace', + scope: 'workspace' + } + ] + }); + } + }); + + assert.ok(resolver.listConnectionSources); + assert.deepEqual( + await resolver.listConnectionSources({ workspace: 'ws-1', provider: 'github' }), + [{ source: { kind: 'workspace' }, providerConfigKey: 'github-relay' }] + ); +}); + +test('connectIntegrations keeps implicit workspace fallback after a compiled JSON round-trip', async () => { + const parsed = parsePersonaSpec({ + id: 'legacy-github-agent', + intent: 'documentation', + tags: ['documentation'], + description: 'legacy default integration source', + harness: 'claude', + model: 'claude-haiku-4-5', + systemPrompt: 'Review changes.', + harnessSettings: { reasoning: 'low', timeoutSeconds: 300 }, + integrations: { github: {} } + }, 'documentation'); + const compiledJson = JSON.stringify(parsed); + assert.equal(compiledJson.includes('__agentworkforceImplicitSource'), false); + assert.equal(compiledJson.includes('"source"'), false); + const reparsed = parsePersonaSpec(JSON.parse(compiledJson), 'documentation'); + let checked = false; + + const result = await connectIntegrations({ + persona: reparsed, + workspace: 'ws-1', + noConnect: false, + noPrompt: true, + io: createBufferedIO(), + integrations: { + async isConnected(args) { + checked = true; + assert.deepEqual(args.source, { kind: 'deployer_user' }); + assert.equal(args.allowWorkspaceFallback, true); + return args.allowWorkspaceFallback === true; + }, + async connect() { + throw new Error('workspace fallback should satisfy preflight'); + } + } + }); + + assert.equal(checked, true); + assert.deepEqual(result.outcomes, [{ provider: 'github', status: 'already-connected' }]); +}); + +test('connectIntegrations explains a source mismatch under --no-prompt', async () => { + const io = createBufferedIO(); + const integrations = relayfileIntegrationResolver({ + apiUrl: 'https://cloud.example.test', + workspaceId: 'ws-1', + workspaceToken: 'workspace-token', + fetch: async (url) => { + const requestUrl = String(url); + if (requestUrl.includes('/integrations/github/status')) { + return okJson({ provider: 'github', configKey: 'github-relay', status: 'pending' }); + } + if (requestUrl.endsWith('/api/v1/me/integrations')) { + return okJson({ error: 'workspace tokens have no user' }, 401); + } + return okJson({ + integrations: [ + { + provider: 'github', + provider_config_key: 'github-relay', + connection_id: 'conn-workspace', + installation_id: 'install-1', + adapter: 'nango', + scope: 'workspace' + } + ] + }); + } + }); + const result = await connectIntegrations({ + persona: { + id: 'github-agent', + intent: 'documentation', + description: 'explicit user source', + tags: ['documentation'], + integrations: { github: { source: { kind: 'deployer_user' } } } + } as never, + workspace: 'ws-1', + noConnect: false, + noPrompt: true, + io, + integrations, + providerConfigKeys: { + async resolve() { + return 'github-relay'; + } + } + }); + + const message = io.messages.find((entry) => entry.level === 'error')?.message ?? ''; + assert.match(message, /required source deployer_user/); + assert.match(message, /Existing sources: workspace/); + assert.match(message, /integrations\.github\.source.*\{"kind":"workspace"\}/); + assert.match(message, /connect github for source deployer_user/); + assert.doesNotMatch(message, /github: not connected/); + assert.match(result.outcomes[0]?.message ?? '', /required source deployer_user/); +}); + +test('connectIntegrations explains a provider-config mismatch under --no-prompt', async () => { + const io = createBufferedIO(); + const result = await connectIntegrations({ + persona: { + id: 'github-agent', + intent: 'documentation', + description: 'explicit user source', + tags: ['documentation'], + integrations: { github: { source: { kind: 'deployer_user' } } } + } as never, + workspace: 'ws-1', + noConnect: false, + noPrompt: true, + io, + integrations: { + async isConnected() { + return false; + }, + async listConnectionSources() { + return [{ + source: { kind: 'deployer_user' }, + providerConfigKey: 'github-existing' + }]; + }, + async connect() { + throw new Error('no-prompt must not connect'); + } + }, + providerConfigKeys: { + async resolve() { + return 'github-required'; + } + } + }); + + const message = io.messages.find((entry) => entry.level === 'error')?.message ?? ''; + assert.match(message, /required provider config "github-required"/); + assert.match(message, /Existing provider configs: github-existing/); + assert.doesNotMatch(message, /Existing sources:/); + assert.match(result.outcomes[0]?.message ?? '', /required provider config/); +}); + +test('connectIntegrations reports a genuinely absent provider connection under --no-prompt', async () => { + const io = createBufferedIO(); + const result = await connectIntegrations({ + persona: { + id: 'github-agent', + intent: 'documentation', + description: 'explicit user source', + tags: ['documentation'], + integrations: { github: { source: { kind: 'deployer_user' } } } + } as never, + workspace: 'ws-1', + noConnect: false, + noPrompt: true, + io, + integrations: { + async isConnected() { + return false; + }, + async listConnectionSources() { + return []; + }, + async connect() { + throw new Error('no-prompt must not connect'); + } + } + }); + + const message = io.messages.find((entry) => entry.level === 'error')?.message ?? ''; + assert.match(message, /no connection exists for provider "github"/); + assert.doesNotMatch(message, /Existing sources:/); + assert.match(result.outcomes[0]?.message ?? '', /no connection exists/); +}); test('relayfileIntegrationResolver isConnected rejects status="error"', async () => { // A failed initial sync or errored writeback means the persona cannot // rely on the integration at dispatch time. Re-prompt OAuth so the user diff --git a/packages/deploy/src/connect.ts b/packages/deploy/src/connect.ts index c2603a78..09383416 100644 --- a/packages/deploy/src/connect.ts +++ b/packages/deploy/src/connect.ts @@ -1,6 +1,10 @@ import { platform } from 'node:os'; import { spawn } from 'node:child_process'; -import type { IntegrationSource, PersonaSpec } from '@agentworkforce/persona-kit'; +import { + isImplicitIntegrationSource, + type IntegrationSource, + type PersonaSpec +} from '@agentworkforce/persona-kit'; import type { DeployIO, IntegrationConnectOutcome } from './types.js'; /** @@ -70,6 +74,15 @@ export interface IntegrationConnectResolver { allowWorkspaceFallback?: boolean; supabaseMcpProjectRef?: string; }): Promise; + /** + * Inspect stored rows for a provider so preflight can distinguish an absent + * connection from one connected at a different source. Optional for custom + * resolvers that cannot enumerate their backing store. + */ + listConnectionSources?(args: { + workspace: string; + provider: string; + }): Promise; /** * Run the browser-based OAuth flow and resolve when the user finishes. * @@ -92,6 +105,11 @@ export interface IntegrationConnectResolver { }): Promise<{ connectionId: string }>; } +export interface IntegrationConnectionLocation { + source: IntegrationSource; + providerConfigKey?: string; +} + /** * Provider linker for `useSubscription: true` personas — connects the * user's chosen LLM provider so cloud inference is billed against their @@ -214,6 +232,42 @@ const fallbackSource = workspaceFallbackSource( expectedConfigKey ); }, + async listConnectionSources({ workspace, provider }) { + const workspaceId = workspace || opts.workspaceId; + const token = await resolveWorkspaceToken(opts.workspaceToken); + const [userResult, workspaceResult] = await Promise.allSettled([ + requestJson( + fetchImpl, + `${apiUrl}/api/v1/me/integrations`, + token, + {}, + sleepImpl + ), + requestJson( + fetchImpl, + `${apiUrl}/api/v1/workspaces/${encodeURIComponent(workspaceId)}/integrations`, + token, + {}, + sleepImpl + ) + ]); + const locations = dedupeConnectionLocations([ + ...(userResult.status === 'fulfilled' + ? connectionLocationsFromList(userResult.value, provider, { kind: 'deployer_user' }) + : []), + ...(workspaceResult.status === 'fulfilled' + ? connectionLocationsFromList(workspaceResult.value, provider, { kind: 'workspace' }) + : []) + ]); + // A CI workspace token may legitimately be unable to call the user-owned + // list. Keep any source rows the workspace list did prove. If the partial + // result is empty, fail the diagnostic instead of asserting that no + // connection exists when one owner could not be inspected. + if (locations.length > 0) return locations; + if (userResult.status === 'rejected') throw userResult.reason; + if (workspaceResult.status === 'rejected') throw workspaceResult.reason; + return []; + }, async connect({ workspace, provider, @@ -797,26 +851,29 @@ export async function connectIntegrations(input: ConnectAllInput): Promise` (which // reaches here when forceReconnect is set). const command = `agent-relay cloud connect ${provider}`; + const captureReason = forceReconnect && connected + ? 'credential recapture was requested' + : connectionProblem; input.io.error( - `integrations.${provider}: not connected. Run \`${command}\` to capture the credential, then re-deploy.` + `integrations.${provider}: ${captureReason}. Run \`${command}\` to capture the credential, then re-deploy.` ); outcomes.push({ provider, status: 'failed', - message: `not connected (run \`${command}\`)` + message: `${captureReason} (run \`${command}\`)` }); continue; } if (!forceReconnect && !input.noPrompt) { + input.io.info(`integrations.${provider}: ${connectionProblem}`); const shouldConnect = await input.io.confirm( `Connect ${provider} now? (opens browser)`, { defaultValue: true } @@ -975,6 +1036,87 @@ async function checkProviderConnected( }); } +async function describeConnectionProblem( + input: ConnectAllInput, + provider: string, + requiredSource: IntegrationSource, + expectedConfigKey: string | undefined +): Promise { + const inspect = input.integrations.listConnectionSources; + if (!inspect) { + return `not connected for required source ${formatIntegrationSource(requiredSource)} (alternate sources could not be inspected)`; + } + + let locations: IntegrationConnectionLocation[]; + try { + locations = await inspect({ workspace: input.workspace, provider }); + } catch (err) { + input.io.warn( + `integrations.${provider}: could not inspect connections at other sources: ${ + err instanceof Error ? err.message : String(err) + }` + ); + return `not connected for required source ${formatIntegrationSource(requiredSource)} (alternate sources could not be inspected)`; + } + + if (locations.length === 0) { + return `no connection exists for provider "${provider}"`; + } + + const configMatches = locations.filter((location) => + !expectedConfigKey + || !location.providerConfigKey + || location.providerConfigKey === expectedConfigKey + ); + if (configMatches.length === 0) { + const existingKeys = uniqueStrings( + locations.map((location) => location.providerConfigKey).filter(isString) + ); + return `connections exist for provider "${provider}", but none use required provider config "${expectedConfigKey}"` + + (existingKeys.length > 0 ? `. Existing provider configs: ${existingKeys.join(', ')}` : ''); + } + + const sourceMatches = configMatches.some((location) => + integrationSourcesEqual(location.source, requiredSource) + ); + const existingSources = uniqueStrings( + configMatches.map((location) => formatIntegrationSource(location.source)) + ); + if (!sourceMatches) { + const authoredSources = uniqueStrings( + configMatches.map((location) => JSON.stringify(location.source)) + ); + return `connections exist for provider "${provider}", but not for required source ${formatIntegrationSource(requiredSource)}. ` + + `Existing sources: ${existingSources.join(', ')}. ` + + `Set \`integrations.${provider}.source\` to ${authoredSources.map((source) => `\`${source}\``).join(' or ')} if that owner is intended, ` + + `or connect ${provider} for source ${formatIntegrationSource(requiredSource)}`; + } + + return `a connection exists for provider "${provider}" at required source ${formatIntegrationSource(requiredSource)}, but it is not ready or does not match the requested configuration`; +} + +function integrationSourcesEqual(a: IntegrationSource, b: IntegrationSource): boolean { + return a.kind === b.kind && ( + a.kind !== 'workspace_service_account' + || b.kind !== 'workspace_service_account' + || a.name === b.name + ); +} + +function formatIntegrationSource(source: IntegrationSource): string { + return source.kind === 'workspace_service_account' + ? `workspace_service_account(${JSON.stringify(source.name)})` + : source.kind; +} + +function uniqueStrings(values: readonly string[]): string[] { + return [...new Set(values)]; +} + +function isString(value: string | undefined): value is string { + return value !== undefined; +} + /** * Error thrown by `requestJson` for any non-2xx response. Carries the numeric * HTTP `status` so callers can branch on it without parsing the message @@ -1299,11 +1441,7 @@ function listHasConnectedProvider( provider: string, opts: MatchOpts = {} ): boolean { - const candidates = Array.isArray(body) - ? body - : body && typeof body === 'object' && Array.isArray((body as { integrations?: unknown }).integrations) - ? (body as { integrations: unknown[] }).integrations - : []; + const candidates = integrationListCandidates(body); return candidates.some((item) => { if (!item || typeof item !== 'object' || Array.isArray(item)) return false; const record = item as Record; @@ -1326,6 +1464,72 @@ function listHasConnectedProvider( }); } +function connectionLocationsFromList( + body: unknown, + provider: string, + fallbackSource: IntegrationSource +): IntegrationConnectionLocation[] { + const locations: IntegrationConnectionLocation[] = []; + for (const item of integrationListCandidates(body)) { + if (!item || typeof item !== 'object' || Array.isArray(item)) continue; + const record = item as Record; + if (readString(record, 'provider') !== provider) continue; + const source = readConnectionSource(record, fallbackSource); + const providerConfigKey = readProviderConfigKey(record); + locations.push({ + source, + ...(providerConfigKey ? { providerConfigKey } : {}) + }); + } + return locations; +} + +function integrationListCandidates(body: unknown): unknown[] { + return Array.isArray(body) + ? body + : body && typeof body === 'object' && Array.isArray((body as { integrations?: unknown }).integrations) + ? (body as { integrations: unknown[] }).integrations + : []; +} + +function readConnectionSource( + record: Record, + fallbackSource: IntegrationSource +): IntegrationSource { + const raw = record.source ?? record.scope; + const kind = typeof raw === 'string' + ? raw + : raw && typeof raw === 'object' && !Array.isArray(raw) + ? readString(raw, 'kind') + : undefined; + if (kind === 'deployer_user' || kind === 'workspace') return { kind }; + if (kind === 'workspace_service_account') { + const name = + (raw && typeof raw === 'object' && !Array.isArray(raw) + ? readString(raw, 'name') + : undefined) + ?? readString(record, 'serviceAccountName') + ?? readString(record, 'name'); + return name ? { kind, name } : fallbackSource; + } + const serviceAccountName = readString(record, 'serviceAccountName'); + return serviceAccountName + ? { kind: 'workspace_service_account', name: serviceAccountName } + : fallbackSource; +} + +function dedupeConnectionLocations( + locations: readonly IntegrationConnectionLocation[] +): IntegrationConnectionLocation[] { + const seen = new Set(); + return locations.filter((location) => { + const key = JSON.stringify(location); + if (seen.has(key)) return false; + seen.add(key); + return true; + }); +} + function statusIsConnectedForSource( status: unknown, provider: string, @@ -1366,6 +1570,7 @@ function readConnectionId(status: unknown): string | undefined { function readProviderConfigKey(value: unknown): string | undefined { return readString(value, 'configKey') ?? readString(value, 'providerConfigKey') + ?? readString(value, 'provider_config_key') ?? readString(value, 'backendIntegrationId'); } @@ -1376,11 +1581,7 @@ function statusMatchesConnectionId(status: unknown, expectedConnectionId: string } function integrationAllowsWorkspaceFallback(value: unknown): boolean { - return Boolean( - value && - typeof value === 'object' && - (value as { __agentworkforceImplicitSource?: unknown }).__agentworkforceImplicitSource === true - ); + return isImplicitIntegrationSource(value); } function isIntegrationListResponse(body: unknown): boolean { diff --git a/packages/deploy/src/deploy.ts b/packages/deploy/src/deploy.ts index 3323f6d2..2728e5c1 100644 --- a/packages/deploy/src/deploy.ts +++ b/packages/deploy/src/deploy.ts @@ -8,7 +8,10 @@ import type { PersonaIntegrationTrigger, PersonaSpec } from '@agentworkforce/persona-kit'; -import { KNOWN_TRIGGER_PROVIDER_ALIASES as TRIGGER_PROVIDER_ALIASES } from '@agentworkforce/persona-kit'; +import { + isImplicitIntegrationSource, + KNOWN_TRIGGER_PROVIDER_ALIASES as TRIGGER_PROVIDER_ALIASES +} from '@agentworkforce/persona-kit'; import { bundleStager } from './bundle.js'; import { resolveCloudUrl } from './cloud-url.js'; import { @@ -610,6 +613,9 @@ function defaultIntegrationResolver(args: { if (await relayfile.isConnected(input).catch(() => false)) return true; return env.isConnected(input); }, + async listConnectionSources(input) { + return await relayfile.listConnectionSources?.(input) ?? []; + }, async connect(input) { return relayfile.connect(input); } @@ -743,11 +749,7 @@ function shouldRequestRuntimeCredentials(args: { } function integrationAllowsWorkspaceFallback(value: unknown): boolean { - return Boolean( - value && - typeof value === 'object' && - (value as { __agentworkforceImplicitSource?: unknown }).__agentworkforceImplicitSource === true - ); + return isImplicitIntegrationSource(value); } function hasByoSandboxEnv(): boolean { diff --git a/packages/persona-kit/src/index.ts b/packages/persona-kit/src/index.ts index 5719af61..9a819362 100644 --- a/packages/persona-kit/src/index.ts +++ b/packages/persona-kit/src/index.ts @@ -115,6 +115,7 @@ export { INPUT_NAME_RE, INTEGRATION_SOURCE_NAME_RE, isHarness, + isImplicitIntegrationSource, isIntent, isObject, isPlainObject, diff --git a/packages/persona-kit/src/parse.test.ts b/packages/persona-kit/src/parse.test.ts index 71ab9dda..02ce24a8 100644 --- a/packages/persona-kit/src/parse.test.ts +++ b/packages/persona-kit/src/parse.test.ts @@ -937,6 +937,16 @@ test('parseIntegrations default-injects source=deployer_user when the persona om true ); assert.equal(Object.keys(i?.github ?? {}).includes('__agentworkforceImplicitSource'), false); + + const serialized = JSON.stringify(i); + assert.equal(serialized, '{"github":{}}'); + assert.equal(serialized.includes('__agentworkforceImplicitSource'), false); + const reparsed = parseIntegrations(JSON.parse(serialized), 'integrations'); + assert.deepEqual(reparsed?.github.source, { kind: 'deployer_user' }); + assert.equal( + (reparsed?.github as { __agentworkforceImplicitSource?: unknown }).__agentworkforceImplicitSource, + true + ); }); test('parseIntegrations round-trips all three valid IntegrationSource kinds', () => { @@ -968,6 +978,16 @@ test('parseIntegrations round-trips all three valid IntegrationSource kinds', () kind: 'workspace_service_account', name: 'release-bot' }); + assert.deepEqual(JSON.parse(JSON.stringify(i)), { + github: { + source: { + kind: 'deployer_user', + futureResolutionPolicy: { fallback: 'workspace' } + } + }, + slack: { source: { kind: 'workspace' } }, + linear: { source: { kind: 'workspace_service_account', name: 'release-bot' } } + }); }); test('parseIntegrations rejects an unknown source.kind with a precise field path', () => { diff --git a/packages/persona-kit/src/parse.ts b/packages/persona-kit/src/parse.ts index 37a6d6cb..19e5c712 100644 --- a/packages/persona-kit/src/parse.ts +++ b/packages/persona-kit/src/parse.ts @@ -49,6 +49,7 @@ import type { * stay short enough to render in list/table UIs without truncation. */ const PERSONA_TAG_MAX_LEN = 64; +const IMPLICIT_INTEGRATION_SOURCE = '__agentworkforceImplicitSource'; export function isObject(value: unknown): value is Record { return typeof value === 'object' && value !== null; @@ -60,6 +61,23 @@ export function isPlainObject(value: unknown): value is Record return proto === Object.prototype || proto === null; } +/** + * Whether an integration is using the backwards-compatible implicit source. + * + * Persona-kit exposes the effective `deployer_user` source to in-memory + * consumers, but serializes an implicit source as an omission. That omission is + * the durable JSON representation: reparsing it restores both the effective + * default and this distinction without leaking private metadata into a persona + * artifact. + */ +export function isImplicitIntegrationSource(value: unknown): boolean { + return Boolean( + isObject(value) && + !Array.isArray(value) && + (value[IMPLICIT_INTEGRATION_SOURCE] === true || value.source === undefined) + ); +} + /** * Copy fields this parser does not own so downstream runtimes can extend the * portable spec without waiting for a persona-kit release. Callers validate @@ -756,17 +774,29 @@ export function parseIntegrationConfig( ]) as PersonaIntegrationConfig; // Default-inject `deployer_user` when the persona omits `source` so - // pre-discriminator personas keep parsing unchanged. The cloud-side - // resolver can then trust `source` is always present on parsed specs. + // pre-discriminator personas keep parsing unchanged. Preserve the omission + // across JSON serialization, though: deploy uses it to distinguish legacy + // workspace-compatible defaults from an explicitly authored deployer-user + // source. A private enumerable marker would leak into persona.json; a private + // non-enumerable marker alone would disappear and lose the behavior. + const sourceWasImplicit = isImplicitIntegrationSource(value); out.source = - source === undefined + sourceWasImplicit ? { kind: 'deployer_user' } : parseIntegrationSource(source, `${context}.source`); - if (source === undefined) { - Object.defineProperty(out, '__agentworkforceImplicitSource', { + if (sourceWasImplicit) { + Object.defineProperty(out, IMPLICIT_INTEGRATION_SOURCE, { value: true, enumerable: false }); + Object.defineProperty(out, 'toJSON', { + enumerable: false, + value(this: PersonaIntegrationConfig) { + const serialized = { ...this }; + delete serialized.source; + return serialized; + } + }); } if (scope !== undefined) { From 40d7c825bf2670816fb66a0c8a3f094ca1e7a0ba Mon Sep 17 00:00:00 2001 From: Ricky Schema Cascade Date: Fri, 4 Sep 2026 19:57:55 +0200 Subject: [PATCH 2/2] fix(deploy): harden preflight diagnostics Review exposed four ways the preflight could still misreport or stall: a serialized reserved marker could override an explicit source, diagnostic list requests were unbounded, the legacy fallback ignored snake-case config keys, and the source helper was absent from the side-effect-free spec entrypoint. Protect authored source intent, keep diagnostics bounded while retaining partial results, and make every supported response shape follow the same config-key matching rules. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BmMcdDakbnDkHBxG4pit7z --- packages/deploy/src/connect.test.ts | 60 ++++++++++++++++++++++++++ packages/deploy/src/connect.ts | 53 ++++++++++++++++++++--- packages/persona-kit/src/parse.test.ts | 24 +++++++++++ packages/persona-kit/src/parse.ts | 12 +++--- packages/persona-kit/src/spec.test.ts | 2 + packages/persona-kit/src/spec.ts | 1 + 6 files changed, 140 insertions(+), 12 deletions(-) diff --git a/packages/deploy/src/connect.test.ts b/packages/deploy/src/connect.test.ts index c6053be2..a1ce247e 100644 --- a/packages/deploy/src/connect.test.ts +++ b/packages/deploy/src/connect.test.ts @@ -403,6 +403,39 @@ test('relayfileIntegrationResolver can diagnose workspace sources with a CI work ); }); +test('relayfileIntegrationResolver bounds source diagnostics and keeps the fulfilled owner', { + timeout: 1_000 +}, async () => { + let stalledSignal: AbortSignal | undefined; + const resolver = relayfileIntegrationResolver({ + apiUrl: 'https://cloud.example.test', + workspaceId: 'ws-1', + workspaceToken: 'workspace-token', + requestTimeoutMs: 10, + fetch: async (url, init) => { + if (String(url).endsWith('/api/v1/me/integrations')) { + stalledSignal = init?.signal ?? undefined; + return await new Promise(() => {}); + } + return okJson({ + integrations: [{ + provider: 'github', + provider_config_key: 'github-relay', + connection_id: 'conn-workspace', + scope: 'workspace' + }] + }); + } + }); + + assert.ok(resolver.listConnectionSources); + assert.deepEqual( + await resolver.listConnectionSources({ workspace: 'ws-1', provider: 'github' }), + [{ source: { kind: 'workspace' }, providerConfigKey: 'github-relay' }] + ); + assert.equal(stalledSignal?.aborted, true); +}); + test('connectIntegrations keeps implicit workspace fallback after a compiled JSON round-trip', async () => { const parsed = parsePersonaSpec({ id: 'legacy-github-agent', @@ -662,6 +695,33 @@ test('relayfileIntegrationResolver isConnected falls back to deployer-user list ); }); +test('relayfileIntegrationResolver fallback list rejects a snake-case config-key mismatch', async () => { + const resolver = relayfileIntegrationResolver({ + apiUrl: 'https://cloud.example.test', + workspaceId: 'ws-1', + workspaceToken: 'tok', + fetch: async (url) => { + if (String(url).includes('/integrations/github/status')) { + return new Response('not found', { status: 404 }); + } + return okJson([{ + provider: 'github', + provider_config_key: 'github-other', + status: 'ready' + }]); + } + }); + + assert.equal( + await resolver.isConnected({ + workspace: 'ws-runtime', + provider: 'github', + expectedConfigKey: 'github-relay' + }), + false + ); +}); + test('relayfileIntegrationResolver fails closed when project-aware Supabase status is unavailable', async () => { const io = createBufferedIO(); const urls: string[] = []; diff --git a/packages/deploy/src/connect.ts b/packages/deploy/src/connect.ts index 09383416..c567c486 100644 --- a/packages/deploy/src/connect.ts +++ b/packages/deploy/src/connect.ts @@ -24,6 +24,7 @@ import type { DeployIO, IntegrationConnectOutcome } from './types.js'; const PROVIDER_ENV_PREFIX = 'WORKFORCE_INTEGRATION_'; const SUPABASE_MCP_PROVIDERS = new Set(['supabase-mcp', 'supabase-mcp-relay']); const SUPABASE_MCP_PROJECT_REF_PATTERN = /^[a-z0-9]{20}$/u; +const DEFAULT_DIAGNOSTIC_REQUEST_TIMEOUT_MS = 10_000; export function normalizeSupabaseMcpProjectRef(value: unknown): string | undefined { if (typeof value !== 'string') return undefined; @@ -159,6 +160,8 @@ export function relayfileIntegrationResolver(opts: { io?: Pick; pollIntervalMs?: number; timeoutMs?: number; + /** Deadline for each best-effort diagnostic list request. Defaults to 10 seconds. */ + requestTimeoutMs?: number; fetch?: typeof fetch; openUrl?: (url: string) => void | Promise; sleep?: (ms: number) => Promise; @@ -236,19 +239,21 @@ const fallbackSource = workspaceFallbackSource( const workspaceId = workspace || opts.workspaceId; const token = await resolveWorkspaceToken(opts.workspaceToken); const [userResult, workspaceResult] = await Promise.allSettled([ - requestJson( + requestJsonWithTimeout( fetchImpl, `${apiUrl}/api/v1/me/integrations`, token, {}, - sleepImpl + sleepImpl, + opts.requestTimeoutMs ?? DEFAULT_DIAGNOSTIC_REQUEST_TIMEOUT_MS ), - requestJson( + requestJsonWithTimeout( fetchImpl, `${apiUrl}/api/v1/workspaces/${encodeURIComponent(workspaceId)}/integrations`, token, {}, - sleepImpl + sleepImpl, + opts.requestTimeoutMs ?? DEFAULT_DIAGNOSTIC_REQUEST_TIMEOUT_MS ) ]); const locations = dedupeConnectionLocations([ @@ -1298,6 +1303,42 @@ async function requestJson( return await res.json(); } +/** + * Bound a diagnostic request even when a custom fetch ignores abort signals. + * The abort still tears down compliant transports; the race guarantees the + * preflight itself settles either way. + */ +async function requestJsonWithTimeout( + fetchImpl: typeof fetch, + url: string, + token: string, + init: RequestInit, + retrySleep: ((ms: number) => Promise) | undefined, + timeoutMs: number +): Promise { + const controller = new AbortController(); + const timeoutError = cloudRequestError( + `cloud integration diagnostic request timed out after ${timeoutMs}ms`, + 408 + ); + let timeout: ReturnType | undefined; + const timeoutPromise = new Promise((_, reject) => { + timeout = setTimeout(() => { + reject(timeoutError); + controller.abort(timeoutError); + }, timeoutMs); + }); + + try { + return await Promise.race([ + requestJson(fetchImpl, url, token, { ...init, signal: controller.signal }, retrySleep), + timeoutPromise + ]); + } finally { + if (timeout) clearTimeout(timeout); + } +} + export function isRetryableIntegrationGet(method: string | undefined): boolean { return method === undefined || method.toUpperCase() === 'GET'; } @@ -1447,8 +1488,8 @@ function listHasConnectedProvider( const record = item as Record; if (record.provider !== provider) return false; if (opts.expectedConfigKey) { - const rowConfigKey = readString(record, 'providerConfigKey'); - // If the row carries a providerConfigKey, enforce strict match. + const rowConfigKey = readProviderConfigKey(record); + // If the row carries a provider config key, 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. diff --git a/packages/persona-kit/src/parse.test.ts b/packages/persona-kit/src/parse.test.ts index 02ce24a8..567e9eb3 100644 --- a/packages/persona-kit/src/parse.test.ts +++ b/packages/persona-kit/src/parse.test.ts @@ -990,6 +990,30 @@ test('parseIntegrations round-trips all three valid IntegrationSource kinds', () }); }); +test('parseIntegrations never trusts an enumerable implicit-source marker', () => { + const markedExplicit = parseIntegrations({ + github: { + source: { kind: 'workspace' }, + __agentworkforceImplicitSource: true + } + }, 'integrations'); + + assert.deepEqual(markedExplicit?.github.source, { kind: 'workspace' }); + assert.equal('__agentworkforceImplicitSource' in (markedExplicit?.github ?? {}), false); + assert.deepEqual(JSON.parse(JSON.stringify(markedExplicit)), { + github: { source: { kind: 'workspace' } } + }); + assert.throws( + () => parseIntegrations({ + github: { + source: { kind: 'org' }, + __agentworkforceImplicitSource: true + } + }, 'integrations'), + /integrations\.github\.source\.kind must be one of/ + ); +}); + test('parseIntegrations rejects an unknown source.kind with a precise field path', () => { assert.throws( () => diff --git a/packages/persona-kit/src/parse.ts b/packages/persona-kit/src/parse.ts index 19e5c712..984ad2d5 100644 --- a/packages/persona-kit/src/parse.ts +++ b/packages/persona-kit/src/parse.ts @@ -71,11 +71,10 @@ export function isPlainObject(value: unknown): value is Record * artifact. */ export function isImplicitIntegrationSource(value: unknown): boolean { - return Boolean( - isObject(value) && - !Array.isArray(value) && - (value[IMPLICIT_INTEGRATION_SOURCE] === true || value.source === undefined) - ); + if (!isObject(value) || Array.isArray(value)) return false; + const marker = Object.getOwnPropertyDescriptor(value, IMPLICIT_INTEGRATION_SOURCE); + return value.source === undefined + || (marker?.value === true && marker.enumerable === false); } /** @@ -770,7 +769,8 @@ export function parseIntegrationConfig( 'config', 'optional', 'enabledByInput', - 'triggers' + 'triggers', + IMPLICIT_INTEGRATION_SOURCE ]) as PersonaIntegrationConfig; // Default-inject `deployer_user` when the persona omits `source` so diff --git a/packages/persona-kit/src/spec.test.ts b/packages/persona-kit/src/spec.test.ts index e4bd71d0..fef7e395 100644 --- a/packages/persona-kit/src/spec.test.ts +++ b/packages/persona-kit/src/spec.test.ts @@ -5,6 +5,7 @@ import { HARNESS_VALUES, deriveAgentCard, isHarness, + isImplicitIntegrationSource, isIntent, parseAgentSpec, parsePersonaSpec @@ -29,6 +30,7 @@ test('spec entrypoint re-exports the validation surface', () => { assert.equal(typeof parseAgentSpec, 'function'); assert.equal(typeof isIntent, 'function'); assert.equal(typeof isHarness, 'function'); + assert.equal(typeof isImplicitIntegrationSource, 'function'); assert.equal(typeof deriveAgentCard, 'function'); assert.ok(Array.isArray(HARNESS_VALUES)); }); diff --git a/packages/persona-kit/src/spec.ts b/packages/persona-kit/src/spec.ts index 52d2c048..2231d5f3 100644 --- a/packages/persona-kit/src/spec.ts +++ b/packages/persona-kit/src/spec.ts @@ -89,6 +89,7 @@ export { INPUT_NAME_RE, INTEGRATION_SOURCE_NAME_RE, isHarness, + isImplicitIntegrationSource, isIntent, isObject, isSidecarMode,