diff --git a/packages/core/src/orchestrator/orchestrator-agent.test.ts b/packages/core/src/orchestrator/orchestrator-agent.test.ts index 1707d99f16..5f2dc35078 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.test.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.test.ts @@ -110,6 +110,7 @@ mock.module('@archon/providers', () => ({ getType: mock(() => 'claude'), getCapabilities: mock(() => ({})), })), + getProviderCapabilities: mock(() => ({ envInjection: true })), })); mock.module('../db/env-vars', () => ({ diff --git a/packages/core/src/orchestrator/orchestrator-agent.ts b/packages/core/src/orchestrator/orchestrator-agent.ts index d9502cce11..856913f38d 100644 --- a/packages/core/src/orchestrator/orchestrator-agent.ts +++ b/packages/core/src/orchestrator/orchestrator-agent.ts @@ -24,7 +24,7 @@ import * as commandHandler from '../handlers/command-handler'; import { formatToolCall } from '@archon/workflows/utils/tool-formatter'; import { classifyAndFormatError } from '../utils/error-formatter'; import { toError } from '../utils/error'; -import { getAgentProvider } from '@archon/providers'; +import { getAgentProvider, getProviderCapabilities } from '@archon/providers'; import { getArchonHome, getArchonWorkspacesPath } from '@archon/paths'; import { syncArchonToWorktree } from '../utils/worktree-sync'; import { syncWorkspace, toRepoPath } from '@archon/git'; @@ -772,6 +772,18 @@ export async function handleMessage( } } const effectiveEnv = { ...(config.envVars ?? {}), ...dbEnvVars }; + + // Warn if provider doesn't support env injection but env vars are configured + if (Object.keys(effectiveEnv).length > 0) { + const providerCaps = getProviderCapabilities(providerKey); + if (!providerCaps.envInjection) { + getLog().warn( + { provider: providerKey, envVarCount: Object.keys(effectiveEnv).length }, + 'orchestrator.unsupported_env_injection' + ); + } + } + const requestOptions: SendQueryOptions = { assistantConfig: (config.assistants[providerKey] ?? {}) as Record, env: Object.keys(effectiveEnv).length > 0 ? effectiveEnv : undefined, diff --git a/packages/providers/src/claude/capabilities.ts b/packages/providers/src/claude/capabilities.ts new file mode 100644 index 0000000000..3874f796ce --- /dev/null +++ b/packages/providers/src/claude/capabilities.ts @@ -0,0 +1,16 @@ +import type { ProviderCapabilities } from '../types'; + +export const CLAUDE_CAPABILITIES: ProviderCapabilities = { + sessionResume: true, + mcp: true, + hooks: true, + skills: true, + toolRestrictions: true, + structuredOutput: true, + envInjection: true, + costControl: true, + effortControl: true, + thinkingControl: true, + fallbackModel: true, + sandbox: true, +}; diff --git a/packages/providers/src/claude/provider.ts b/packages/providers/src/claude/provider.ts index 57e430579b..b4769e66ec 100644 --- a/packages/providers/src/claude/provider.ts +++ b/packages/providers/src/claude/provider.ts @@ -28,6 +28,7 @@ import type { NodeConfig, } from '../types'; import { parseClaudeConfig } from './config'; +import { CLAUDE_CAPABILITIES } from './capabilities'; import { createLogger } from '@archon/paths'; import { readFile } from 'fs/promises'; import { resolve, isAbsolute } from 'path'; @@ -819,20 +820,7 @@ export class ClaudeProvider implements IAgentProvider { } getCapabilities(): ProviderCapabilities { - return { - sessionResume: true, - mcp: true, - hooks: true, - skills: true, - toolRestrictions: true, - structuredOutput: true, - envInjection: true, - costControl: true, - effortControl: true, - thinkingControl: true, - fallbackModel: true, - sandbox: true, - }; + return CLAUDE_CAPABILITIES; } /** diff --git a/packages/providers/src/codex/capabilities.ts b/packages/providers/src/codex/capabilities.ts new file mode 100644 index 0000000000..03cc0773cf --- /dev/null +++ b/packages/providers/src/codex/capabilities.ts @@ -0,0 +1,16 @@ +import type { ProviderCapabilities } from '../types'; + +export const CODEX_CAPABILITIES: ProviderCapabilities = { + sessionResume: true, + mcp: false, + hooks: false, + skills: false, + toolRestrictions: false, + structuredOutput: true, + envInjection: true, + costControl: false, + effortControl: false, + thinkingControl: false, + fallbackModel: false, + sandbox: false, +}; diff --git a/packages/providers/src/codex/provider.ts b/packages/providers/src/codex/provider.ts index fb2a8b2790..b9e1d493e9 100644 --- a/packages/providers/src/codex/provider.ts +++ b/packages/providers/src/codex/provider.ts @@ -16,6 +16,7 @@ import type { ProviderCapabilities, } from '../types'; import { parseCodexConfig } from './config'; +import { CODEX_CAPABILITIES } from './capabilities'; import { resolveCodexBinaryPath } from './binary-resolver'; import { createLogger } from '@archon/paths'; @@ -496,20 +497,7 @@ export class CodexProvider implements IAgentProvider { } getCapabilities(): ProviderCapabilities { - return { - sessionResume: true, - mcp: false, - hooks: false, - skills: false, - toolRestrictions: false, - structuredOutput: true, - envInjection: true, - costControl: false, - effortControl: false, - thinkingControl: false, - fallbackModel: false, - sandbox: false, - }; + return CODEX_CAPABILITIES; } async *sendQuery( diff --git a/packages/providers/src/factory.test.ts b/packages/providers/src/factory.test.ts index fcc62c09a6..86fa4a3420 100644 --- a/packages/providers/src/factory.test.ts +++ b/packages/providers/src/factory.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect } from 'bun:test'; -import { getAgentProvider } from './factory'; +import { getAgentProvider, getProviderCapabilities } from './factory'; import { UnknownProviderError } from './errors'; describe('factory', () => { @@ -62,4 +62,44 @@ describe('factory', () => { expect(codexCaps.hooks).toBe(false); }); }); + + describe('getProviderCapabilities', () => { + test('returns Claude capabilities without instantiation', () => { + const caps = getProviderCapabilities('claude'); + expect(caps.mcp).toBe(true); + expect(caps.hooks).toBe(true); + expect(caps.envInjection).toBe(true); + }); + + test('returns Codex capabilities without instantiation', () => { + const caps = getProviderCapabilities('codex'); + expect(caps.mcp).toBe(false); + expect(caps.hooks).toBe(false); + expect(caps.envInjection).toBe(true); + }); + + test('matches runtime getCapabilities for Claude', () => { + const staticCaps = getProviderCapabilities('claude'); + const runtimeCaps = getAgentProvider('claude').getCapabilities(); + expect(staticCaps).toEqual(runtimeCaps); + }); + + test('matches runtime getCapabilities for Codex', () => { + const staticCaps = getProviderCapabilities('codex'); + const runtimeCaps = getAgentProvider('codex').getCapabilities(); + expect(staticCaps).toEqual(runtimeCaps); + }); + + test('throws UnknownProviderError for unknown type', () => { + expect(() => getProviderCapabilities('unknown')).toThrow(UnknownProviderError); + }); + + test('throws UnknownProviderError for empty string', () => { + expect(() => getProviderCapabilities('')).toThrow(UnknownProviderError); + }); + + test('is case sensitive - Claude throws', () => { + expect(() => getProviderCapabilities('Claude')).toThrow(UnknownProviderError); + }); + }); }); diff --git a/packages/providers/src/factory.ts b/packages/providers/src/factory.ts index 836f3edce5..bcd15eb9b1 100644 --- a/packages/providers/src/factory.ts +++ b/packages/providers/src/factory.ts @@ -1,12 +1,14 @@ /** * Agent Provider Factory * - * Dynamically instantiates the appropriate agent provider based on type string. + * Dynamic provider instantiation and static capability lookup. * Built-in providers only: Claude and Codex. */ -import type { IAgentProvider } from './types'; +import type { IAgentProvider, ProviderCapabilities } from './types'; import { ClaudeProvider } from './claude/provider'; import { CodexProvider } from './codex/provider'; +import { CLAUDE_CAPABILITIES } from './claude/capabilities'; +import { CODEX_CAPABILITIES } from './codex/capabilities'; import { UnknownProviderError } from './errors'; import { createLogger } from '@archon/paths'; @@ -39,3 +41,18 @@ export function getAgentProvider(type: string): IAgentProvider { throw new UnknownProviderError(type, [...REGISTERED_PROVIDERS]); } } + +/** + * Get provider capabilities without instantiating a provider. + * Used by dag-executor and orchestrator for capability warnings. + */ +export function getProviderCapabilities(type: string): ProviderCapabilities { + switch (type) { + case 'claude': + return CLAUDE_CAPABILITIES; + case 'codex': + return CODEX_CAPABILITIES; + default: + throw new UnknownProviderError(type, [...REGISTERED_PROVIDERS]); + } +} diff --git a/packages/providers/src/index.ts b/packages/providers/src/index.ts index b46cb84111..6bafb1da00 100644 --- a/packages/providers/src/index.ts +++ b/packages/providers/src/index.ts @@ -13,7 +13,9 @@ export type { // Import from ./types directly or from the config modules — both work. // Factory -export { getAgentProvider } from './factory'; +export { getAgentProvider, getProviderCapabilities } from './factory'; +// Static capability constants are intentionally NOT re-exported here. +// Use getProviderCapabilities() instead — it's the correct public seam. // Error export { UnknownProviderError } from './errors'; diff --git a/packages/workflows/src/dag-executor.ts b/packages/workflows/src/dag-executor.ts index b2488a70f2..2db7cdef28 100644 --- a/packages/workflows/src/dag-executor.ts +++ b/packages/workflows/src/dag-executor.ts @@ -20,6 +20,7 @@ import type { ProviderCapabilities, TokenUsage, } from '@archon/providers/types'; +import { getProviderCapabilities } from '@archon/providers'; import type { DagNode, ApprovalNode, @@ -47,7 +48,7 @@ import { formatToolCall } from './utils/tool-formatter'; import { createLogger } from '@archon/paths'; import { getWorkflowEventEmitter } from './event-emitter'; import { evaluateCondition } from './condition-evaluator'; -import { isClaudeModel, isModelCompatible } from './model-validation'; +import { inferProviderFromModel, isModelCompatible } from './model-validation'; import { logNodeStart, logNodeComplete, @@ -250,24 +251,14 @@ async function resolveNodeProviderAndModel( conversationId: string, workflowRunId: string, _cwd: string, - workflowLevelOptions: WorkflowLevelOptions, - deps: WorkflowDeps + workflowLevelOptions: WorkflowLevelOptions ): Promise<{ provider: 'claude' | 'codex'; model: string | undefined; options: SendQueryOptions | undefined; }> { - let provider: 'claude' | 'codex'; - - if (node.provider) { - provider = node.provider; - } else if (node.model && isClaudeModel(node.model)) { - provider = 'claude'; - } else if (node.model) { - provider = 'codex'; - } else { - provider = workflowProvider; - } + const provider: 'claude' | 'codex' = + node.provider ?? inferProviderFromModel(node.model, workflowProvider); const model = node.model ?? @@ -279,9 +270,8 @@ async function resolveNodeProviderAndModel( ); } - // Get provider capabilities for capability warnings - const aiClient = deps.getAgentProvider(provider); - const caps = aiClient.getCapabilities(); + // Get provider capabilities for capability warnings (static lookup, no instantiation) + const caps = getProviderCapabilities(provider); // Capability warnings — inform users when features are unsupported const capChecks: [string, keyof ProviderCapabilities, boolean][] = [ @@ -2040,8 +2030,7 @@ async function executeApprovalNode( conversationId, workflowRun.id, cwd, - workflowLevelOptions, - deps + workflowLevelOptions ); const output = await executeNodeInternal( @@ -2360,16 +2349,8 @@ export async function executeDagWorkflow( // 3b. Loop node dispatch — manages its own AI sessions and iteration if (isLoopNode(node)) { // Resolve per-node provider/model overrides (same logic as other node types) - let loopProvider: 'claude' | 'codex'; - if (node.provider) { - loopProvider = node.provider; - } else if (node.model && isClaudeModel(node.model)) { - loopProvider = 'claude'; - } else if (node.model) { - loopProvider = 'codex'; - } else { - loopProvider = workflowProvider; - } + const loopProvider: 'claude' | 'codex' = + node.provider ?? inferProviderFromModel(node.model, workflowProvider); const loopModel = node.model ?? (loopProvider === workflowProvider @@ -2494,8 +2475,7 @@ export async function executeDagWorkflow( conversationId, workflowRun.id, cwd, - workflowLevelOptions, - deps + workflowLevelOptions ); // 5. Determine session — parallel or context:fresh → always fresh diff --git a/packages/workflows/src/executor.ts b/packages/workflows/src/executor.ts index e87ea9065b..6e7dee750c 100644 --- a/packages/workflows/src/executor.ts +++ b/packages/workflows/src/executor.ts @@ -12,7 +12,7 @@ import type { WorkflowDefinition, WorkflowRun, WorkflowExecutionResult } from '. import { executeDagWorkflow } from './dag-executor'; import { logWorkflowStart, logWorkflowError } from './logger'; import { getWorkflowEventEmitter } from './event-emitter'; -import { isClaudeModel, isModelCompatible } from './model-validation'; +import { inferProviderFromModel, isModelCompatible } from './model-validation'; import { classifyError } from './executor-shared'; /** Lazy-initialized logger (deferred so test mocks can intercept createLogger) */ @@ -283,11 +283,8 @@ export async function executeWorkflow( if (workflow.provider) { resolvedProvider = workflow.provider; providerSource = 'workflow definition'; - } else if (workflow.model && isClaudeModel(workflow.model)) { - resolvedProvider = 'claude'; - providerSource = 'inferred from workflow model'; } else if (workflow.model) { - resolvedProvider = 'codex'; + resolvedProvider = inferProviderFromModel(workflow.model, config.assistant); providerSource = 'inferred from workflow model'; } else { resolvedProvider = config.assistant; diff --git a/packages/workflows/src/model-validation.test.ts b/packages/workflows/src/model-validation.test.ts index a73b7586aa..b3663b804e 100644 --- a/packages/workflows/src/model-validation.test.ts +++ b/packages/workflows/src/model-validation.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'bun:test'; -import { isClaudeModel, isModelCompatible } from './model-validation'; +import { isClaudeModel, isModelCompatible, inferProviderFromModel } from './model-validation'; describe('model-validation', () => { describe('isClaudeModel', () => { @@ -66,4 +66,30 @@ describe('model-validation', () => { expect(isModelCompatible('codex', '')).toBe(true); }); }); + + describe('inferProviderFromModel', () => { + it('should return default when model is undefined', () => { + expect(inferProviderFromModel(undefined, 'claude')).toBe('claude'); + expect(inferProviderFromModel(undefined, 'codex')).toBe('codex'); + }); + + it('should return default when model is empty string', () => { + expect(inferProviderFromModel('', 'claude')).toBe('claude'); + expect(inferProviderFromModel('', 'codex')).toBe('codex'); + }); + + it('should infer claude from Claude model names', () => { + expect(inferProviderFromModel('sonnet', 'codex')).toBe('claude'); + expect(inferProviderFromModel('opus', 'codex')).toBe('claude'); + expect(inferProviderFromModel('haiku', 'codex')).toBe('claude'); + expect(inferProviderFromModel('inherit', 'codex')).toBe('claude'); + expect(inferProviderFromModel('claude-opus-4-6', 'codex')).toBe('claude'); + }); + + it('should infer codex from non-Claude model names', () => { + expect(inferProviderFromModel('gpt-5.3-codex', 'claude')).toBe('codex'); + expect(inferProviderFromModel('gpt-4', 'claude')).toBe('codex'); + expect(inferProviderFromModel('o1-mini', 'claude')).toBe('codex'); + }); + }); }); diff --git a/packages/workflows/src/model-validation.ts b/packages/workflows/src/model-validation.ts index b035582717..a88a700481 100644 --- a/packages/workflows/src/model-validation.ts +++ b/packages/workflows/src/model-validation.ts @@ -8,6 +8,24 @@ export function isClaudeModel(model: string): boolean { ); } +/** + * Infer provider from a model name. Returns 'claude' if the model matches + * Claude naming patterns, 'codex' otherwise. + * + * When no model is provided, returns the default provider. + * + * Phase 2 will replace this with a registry-driven lookup that iterates + * built-in provider registrations. + */ +export function inferProviderFromModel( + model: string | undefined, + defaultProvider: 'claude' | 'codex' +): 'claude' | 'codex' { + if (!model) return defaultProvider; + if (isClaudeModel(model)) return 'claude'; + return 'codex'; +} + export function isModelCompatible(provider: 'claude' | 'codex', model?: string): boolean { if (!model) return true; if (provider === 'claude') return isClaudeModel(model);