mirror of
https://github.com/coleam00/Archon
synced 2026-04-21 13:37:41 +00:00
fix(workflows): stop warning about model/provider on loop nodes (#1090)
* fix(workflows): stop warning about model/provider on loop nodes (#1082)
The loader incorrectly classified loop nodes as "non-AI nodes" and warned
that model/provider fields were ignored, even though the DAG executor has
supported these fields on loop nodes since commit 594d5daa.
Changes:
- Add LOOP_NODE_AI_FIELDS constant excluding model/provider from the warn list
- Update loader to use LOOP_NODE_AI_FIELDS for loop node field checking
- Fix BASH_NODE_AI_FIELDS comment that incorrectly referenced loop nodes
- Add tests for loop node model/provider acceptance and unsupported field warnings
Fixes #1082
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(workflows): update stale comment and add LOOP_NODE_AI_FIELDS unit tests
- Update section comment from "bash/loop nodes" to "non-AI nodes" since loop
nodes do support model/provider (the fix in this PR)
- Export LOOP_NODE_AI_FIELDS from schemas/index.ts alongside BASH/SCRIPT variants
- Add dedicated describe block in schemas.test.ts verifying that model and
provider are excluded and all other BASH_NODE_AI_FIELDS are still present
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* simplify: merge nodeType and aiFields into a single if/else chain in parseDagNode
Eliminates the separate isNonAiNode predicate and nested ternary for aiFields
selection by combining both into one explicit if/else block — each branch sets
nodeType and aiFields together, removing the need to re-check node type twice.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
64bdd30ef4
commit
818854474f
5 changed files with 147 additions and 23 deletions
|
|
@ -1282,6 +1282,82 @@ nodes:
|
|||
expect(node.provider).toBeUndefined();
|
||||
expect(node.model).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should NOT warn about model/provider on loop nodes (they are supported)', async () => {
|
||||
const workflowDir = join(testDir, '.archon', 'workflows');
|
||||
await mkdir(workflowDir, { recursive: true });
|
||||
|
||||
await writeFile(
|
||||
join(workflowDir, 'loop-model.yaml'),
|
||||
`
|
||||
name: loop-model
|
||||
description: Loop with model override
|
||||
nodes:
|
||||
- id: iterate
|
||||
loop:
|
||||
prompt: "Do something"
|
||||
until: "COMPLETE"
|
||||
max_iterations: 3
|
||||
provider: claude
|
||||
model: claude-opus-4-6
|
||||
`
|
||||
);
|
||||
|
||||
(mockLogger.warn as Mock<() => undefined>).mockClear();
|
||||
const result = await discoverWorkflows(testDir, { loadDefaults: false });
|
||||
expect(result.errors).toHaveLength(0);
|
||||
expect(result.workflows).toHaveLength(1);
|
||||
|
||||
const node = result.workflows[0].workflow.nodes[0];
|
||||
expect(isLoopNode(node)).toBe(true);
|
||||
|
||||
// model and provider should NOT trigger a warning
|
||||
const warnCalls = (mockLogger.warn as Mock<() => undefined>).mock.calls;
|
||||
const aiFieldWarnings = warnCalls.filter(
|
||||
call => typeof call[1] === 'string' && call[1].includes('ai_fields_ignored')
|
||||
);
|
||||
expect(aiFieldWarnings).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should warn about unsupported AI fields on loop nodes (not model/provider)', async () => {
|
||||
const workflowDir = join(testDir, '.archon', 'workflows');
|
||||
await mkdir(workflowDir, { recursive: true });
|
||||
|
||||
await writeFile(
|
||||
join(workflowDir, 'loop-unsupported.yaml'),
|
||||
`
|
||||
name: loop-unsupported
|
||||
description: Loop with unsupported AI fields
|
||||
nodes:
|
||||
- id: iterate
|
||||
loop:
|
||||
prompt: "Do something"
|
||||
until: "COMPLETE"
|
||||
max_iterations: 3
|
||||
model: claude-opus-4-6
|
||||
output_format:
|
||||
type: object
|
||||
properties:
|
||||
status:
|
||||
type: string
|
||||
`
|
||||
);
|
||||
|
||||
(mockLogger.warn as Mock<() => undefined>).mockClear();
|
||||
const result = await discoverWorkflows(testDir, { loadDefaults: false });
|
||||
expect(result.errors).toHaveLength(0);
|
||||
|
||||
// Should warn about output_format but NOT about model
|
||||
const warnCalls = (mockLogger.warn as Mock<() => undefined>).mock.calls;
|
||||
const aiFieldWarnings = warnCalls.filter(
|
||||
call => typeof call[1] === 'string' && call[1].includes('ai_fields_ignored')
|
||||
);
|
||||
expect(aiFieldWarnings).toHaveLength(1);
|
||||
const warnedFields = (aiFieldWarnings[0][0] as { fields: string[] }).fields;
|
||||
expect(warnedFields).toContain('output_format');
|
||||
expect(warnedFields).not.toContain('model');
|
||||
expect(warnedFields).not.toContain('provider');
|
||||
});
|
||||
});
|
||||
|
||||
describe('DAG output ref validation', () => {
|
||||
|
|
|
|||
|
|
@ -5,7 +5,12 @@ import type { WorkflowDefinition, WorkflowLoadError, DagNode, WorkflowNodeHooks
|
|||
import { isLoopNode, isApprovalNode, isCancelNode, isScriptNode } from './schemas';
|
||||
import { createLogger } from '@archon/paths';
|
||||
import { isModelCompatible } from './model-validation';
|
||||
import { dagNodeSchema, BASH_NODE_AI_FIELDS, SCRIPT_NODE_AI_FIELDS } from './schemas/dag-node';
|
||||
import {
|
||||
dagNodeSchema,
|
||||
BASH_NODE_AI_FIELDS,
|
||||
SCRIPT_NODE_AI_FIELDS,
|
||||
LOOP_NODE_AI_FIELDS,
|
||||
} from './schemas/dag-node';
|
||||
import { modelReasoningEffortSchema, webSearchModeSchema } from './schemas/workflow';
|
||||
import { workflowNodeHooksSchema } from './schemas/hooks';
|
||||
import { z } from '@hono/zod-openapi';
|
||||
|
|
@ -56,26 +61,25 @@ function parseDagNode(raw: unknown, index: number, errors: string[]): DagNode |
|
|||
const node = result.data;
|
||||
|
||||
// Warn about AI-specific fields on non-AI nodes (runtime behavior, not schema errors)
|
||||
const isNonAiNode =
|
||||
('bash' in node && typeof node.bash === 'string') ||
|
||||
isScriptNode(node) ||
|
||||
isLoopNode(node) ||
|
||||
isApprovalNode(node) ||
|
||||
isCancelNode(node);
|
||||
if (isNonAiNode) {
|
||||
let nodeType: string;
|
||||
if (isCancelNode(node)) {
|
||||
nodeType = 'cancel';
|
||||
} else if (isApprovalNode(node)) {
|
||||
nodeType = 'approval';
|
||||
} else if (isLoopNode(node)) {
|
||||
nodeType = 'loop';
|
||||
} else if (isScriptNode(node)) {
|
||||
nodeType = 'script';
|
||||
} else {
|
||||
nodeType = 'bash';
|
||||
}
|
||||
const aiFields = isScriptNode(node) ? SCRIPT_NODE_AI_FIELDS : BASH_NODE_AI_FIELDS;
|
||||
let nodeType: string | undefined;
|
||||
let aiFields: readonly string[] | undefined;
|
||||
if (isCancelNode(node)) {
|
||||
nodeType = 'cancel';
|
||||
aiFields = BASH_NODE_AI_FIELDS;
|
||||
} else if (isApprovalNode(node)) {
|
||||
nodeType = 'approval';
|
||||
aiFields = BASH_NODE_AI_FIELDS;
|
||||
} else if (isLoopNode(node)) {
|
||||
nodeType = 'loop';
|
||||
aiFields = LOOP_NODE_AI_FIELDS;
|
||||
} else if (isScriptNode(node)) {
|
||||
nodeType = 'script';
|
||||
aiFields = SCRIPT_NODE_AI_FIELDS;
|
||||
} else if ('bash' in node && typeof node.bash === 'string') {
|
||||
nodeType = 'bash';
|
||||
aiFields = BASH_NODE_AI_FIELDS;
|
||||
}
|
||||
if (nodeType !== undefined && aiFields !== undefined) {
|
||||
const presentAiFields = aiFields.filter(f => (raw as Record<string, unknown>)[f] !== undefined);
|
||||
if (presentAiFields.length > 0) {
|
||||
getLog().warn({ id: node.id, fields: presentAiFields }, `${nodeType}_node_ai_fields_ignored`);
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import {
|
|||
isTriggerRule,
|
||||
TRIGGER_RULES,
|
||||
SCRIPT_NODE_AI_FIELDS,
|
||||
LOOP_NODE_AI_FIELDS,
|
||||
approvalOnRejectSchema,
|
||||
dagNodeSchema,
|
||||
} from './schemas';
|
||||
|
|
@ -661,3 +662,36 @@ describe('SCRIPT_NODE_AI_FIELDS', () => {
|
|||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// LOOP_NODE_AI_FIELDS constant
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('LOOP_NODE_AI_FIELDS', () => {
|
||||
test('excludes model and provider (loop nodes support them)', () => {
|
||||
expect(LOOP_NODE_AI_FIELDS).not.toContain('model');
|
||||
expect(LOOP_NODE_AI_FIELDS).not.toContain('provider');
|
||||
});
|
||||
|
||||
test('contains all other AI-specific fields from BASH_NODE_AI_FIELDS', () => {
|
||||
const expectedFields = [
|
||||
'context',
|
||||
'output_format',
|
||||
'allowed_tools',
|
||||
'denied_tools',
|
||||
'hooks',
|
||||
'mcp',
|
||||
'skills',
|
||||
'effort',
|
||||
'thinking',
|
||||
'maxBudgetUsd',
|
||||
'systemPrompt',
|
||||
'fallbackModel',
|
||||
'betas',
|
||||
'sandbox',
|
||||
];
|
||||
for (const field of expectedFields) {
|
||||
expect(LOOP_NODE_AI_FIELDS).toContain(field);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -291,10 +291,10 @@ export type DagNode =
|
|||
| ScriptNode;
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// AI-specific fields that are meaningless on bash/loop nodes
|
||||
// AI-specific fields that are meaningless on non-AI nodes
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/** AI-specific fields that are meaningless on bash/loop nodes — exported for loader warnings */
|
||||
/** AI-specific fields that are meaningless on bash nodes — exported for loader warnings */
|
||||
export const BASH_NODE_AI_FIELDS: readonly string[] = [
|
||||
'provider',
|
||||
'model',
|
||||
|
|
@ -317,6 +317,15 @@ export const BASH_NODE_AI_FIELDS: readonly string[] = [
|
|||
/** AI-specific fields that are meaningless on script nodes — same as bash nodes */
|
||||
export const SCRIPT_NODE_AI_FIELDS: readonly string[] = BASH_NODE_AI_FIELDS;
|
||||
|
||||
/**
|
||||
* AI-specific fields that are unsupported on loop nodes.
|
||||
* `model` and `provider` are excluded because the DAG executor resolves and
|
||||
* forwards them to each iteration's AI call (see dag-executor.ts:2602-2648).
|
||||
*/
|
||||
export const LOOP_NODE_AI_FIELDS: readonly string[] = BASH_NODE_AI_FIELDS.filter(
|
||||
f => f !== 'model' && f !== 'provider'
|
||||
);
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// dagNodeSchema — flat validation schema with transform to DagNode
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -47,6 +47,7 @@ export {
|
|||
isTriggerRule,
|
||||
BASH_NODE_AI_FIELDS,
|
||||
SCRIPT_NODE_AI_FIELDS,
|
||||
LOOP_NODE_AI_FIELDS,
|
||||
effortLevelSchema,
|
||||
thinkingConfigSchema,
|
||||
sandboxSettingsSchema,
|
||||
|
|
|
|||
Loading…
Reference in a new issue