From 2bbab8cd07bb6a08f673a211f1a0c6e68ef55ae6 Mon Sep 17 00:00:00 2001 From: "puneet.monga" Date: Thu, 14 May 2026 18:37:23 -0700 Subject: [PATCH] fix(lint): require target on ActionBlock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An action declared with only a description (no target, no body) was silently accepted by lint and compiler. Reasoning could bind to it, and at runtime the call was a no-op because nothing wired it to a real implementation. Mark ActionBlock.target as .required() so the existing requiredFieldPass emits a missing-required-field diagnostic when it is absent. No new lint pass is needed — the schema-level signal plugs into machinery that already exists. Adds colocated unit tests covering the missing/present/multiple cases. --- packages/language/src/blocks.ts | 2 +- .../language/src/lint/required-fields.test.ts | 99 +++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 packages/language/src/lint/required-fields.test.ts diff --git a/packages/language/src/blocks.ts b/packages/language/src/blocks.ts index 3aeea21..4843e07 100644 --- a/packages/language/src/blocks.ts +++ b/packages/language/src/blocks.ts @@ -118,7 +118,7 @@ export const ActionBlock = NamedBlock( label: StringValue.describe('Display label shown in the UI.'), inputs: InputsBlock, outputs: OutputsBlock, - target: StringValue.describe( + target: StringValue.required().describe( 'External implementation target URI (e.g., "flow://Action_Name").' ), source: StringValue.describe( diff --git a/packages/language/src/lint/required-fields.test.ts b/packages/language/src/lint/required-fields.test.ts new file mode 100644 index 0000000..ab0f164 --- /dev/null +++ b/packages/language/src/lint/required-fields.test.ts @@ -0,0 +1,99 @@ +/* + * Copyright (c) 2026, Salesforce, Inc. + * All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * For full license text, see the LICENSE file in the repo root or https://www.apache.org/licenses/LICENSE-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { parse } from '@agentscript/parser'; +import { Dialect } from '../core/dialect.js'; +import { LintEngine } from '../core/analysis/lint-engine.js'; +import { createSchemaContext } from '../core/analysis/scope.js'; +import { ActionsBlock } from '../blocks.js'; +import { requiredFieldPass } from './required-fields.js'; + +const TestSchema = { + actions: ActionsBlock, +}; + +const schemaCtx = createSchemaContext({ schema: TestSchema, aliases: {} }); + +function getDiagnostics(source: string, code?: string) { + const { rootNode: root } = parse(source); + const mappingNode = + root.namedChildren.find(n => n.type === 'mapping') ?? root; + + const dialect = new Dialect(); + const result = dialect.parse(mappingNode, TestSchema); + + const engine = new LintEngine({ + passes: [requiredFieldPass()], + source: 'test', + }); + const { diagnostics } = engine.run(result.value, schemaCtx); + if (!code) return diagnostics; + return diagnostics.filter(d => d.code === code); +} + +describe('required target on ActionBlock', () => { + it('flags an action that has only a description', () => { + const diags = getDiagnostics( + ` +actions: + Notify_Manager: + description: "Notify the store manager" +`, + 'missing-required-field' + ); + expect(diags).toHaveLength(1); + expect(diags[0].message).toContain('target'); + }); + + it('does not flag an action that declares target', () => { + const diags = getDiagnostics( + ` +actions: + Notify_Manager: + description: "Notify the store manager" + target: "flow://Notify_Manager" +`, + 'missing-required-field' + ); + expect(diags).toHaveLength(0); + }); + + it('flags an action with inputs/outputs but no target', () => { + const diags = getDiagnostics( + ` +actions: + Lookup: + description: "Look something up" + inputs: + query: string + outputs: + result: string +`, + 'missing-required-field' + ); + expect(diags).toHaveLength(1); + expect(diags[0].message).toContain('target'); + }); + + it('reports one diagnostic per action when multiple are missing target', () => { + const diags = getDiagnostics( + ` +actions: + Alpha: + description: "alpha" + Beta: + description: "beta" + target: "flow://Beta" + Gamma: + description: "gamma" +`, + 'missing-required-field' + ); + expect(diags).toHaveLength(2); + }); +});