From 2e46c66878d83fd4ff2b9b19124b97bbf73cea05 Mon Sep 17 00:00:00 2001 From: eric sciple Date: Tue, 6 Jan 2026 21:09:38 -0600 Subject: [PATCH 1/3] Context-aware autocomplete and validation for action.yml runs section (#289) - Set main as required in node-runs-strict schema definition - Add validation for invalid key combinations based on using value - Add validation for missing required keys (main for node, steps for composite, image for docker) - Filter autocomplete suggestions based on using value - Prioritize 'using' in completions when not set yet Fixes context-aware autocomplete for action.yml files where different action types (node, composite, docker) have different valid keys under runs: --- languageservice/src/complete-action.test.ts | 78 ++++++++++++ languageservice/src/complete.ts | 112 ++++++++++++++++- languageservice/src/validate-action.test.ts | 81 ++++++++++++ languageservice/src/validate-action.ts | 132 ++++++++++++++++++++ workflow-parser/src/action-v1.0.json | 1 + 5 files changed, 403 insertions(+), 1 deletion(-) diff --git a/languageservice/src/complete-action.test.ts b/languageservice/src/complete-action.test.ts index 8f7cca9..76340f4 100644 --- a/languageservice/src/complete-action.test.ts +++ b/languageservice/src/complete-action.test.ts @@ -184,6 +184,84 @@ runs: expect(labels).toContain("using"); }); + + it("filters runs keys for node20 actions", async () => { + const [doc, position] = createActionDocument(`name: Test +description: Test +runs: + using: node20 + |`); + const completions = await complete(doc, position); + const labels = completions.map(c => c.label); + + // Should show Node.js action keys + expect(labels).toContain("main"); + expect(labels).toContain("pre"); + expect(labels).toContain("post"); + expect(labels).toContain("pre-if"); + expect(labels).toContain("post-if"); + + // Should NOT show composite or docker keys + expect(labels).not.toContain("steps"); + expect(labels).not.toContain("image"); + expect(labels).not.toContain("entrypoint"); + }); + + it("filters runs keys for composite actions", async () => { + const [doc, position] = createActionDocument(`name: Test +description: Test +runs: + using: composite + |`); + const completions = await complete(doc, position); + const labels = completions.map(c => c.label); + + // Should show composite action keys + expect(labels).toContain("steps"); + + // Should NOT show Node.js or docker keys + expect(labels).not.toContain("main"); + expect(labels).not.toContain("pre"); + expect(labels).not.toContain("post"); + expect(labels).not.toContain("image"); + }); + + it("filters runs keys for docker actions", async () => { + const [doc, position] = createActionDocument(`name: Test +description: Test +runs: + using: docker + |`); + const completions = await complete(doc, position); + const labels = completions.map(c => c.label); + + // Should show Docker action keys + expect(labels).toContain("image"); + expect(labels).toContain("args"); + expect(labels).toContain("env"); + expect(labels).toContain("entrypoint"); + expect(labels).toContain("pre-entrypoint"); + expect(labels).toContain("post-entrypoint"); + + // Should NOT show Node.js or composite keys + expect(labels).not.toContain("main"); + expect(labels).not.toContain("steps"); + }); + + it("prioritizes using when not set", async () => { + const [doc, position] = createActionDocument(`name: Test +description: Test +runs: + |`); + const completions = await complete(doc, position); + + // Find the using completion + const usingCompletion = completions.find(c => c.label === "using"); + expect(usingCompletion).toBeDefined(); + + // It should have a sortText that makes it sort first + expect(usingCompletion?.sortText).toBe("0_using"); + }); }); describe("branding completions", () => { diff --git a/languageservice/src/complete.ts b/languageservice/src/complete.ts index 720b9d6..8b05356 100644 --- a/languageservice/src/complete.ts +++ b/languageservice/src/complete.ts @@ -38,6 +38,24 @@ import {Value, ValueProviderConfig} from "./value-providers/config.js"; import {defaultValueProviders} from "./value-providers/default.js"; import {DefinitionValueMode, definitionValues, TokenStructure} from "./value-providers/definition.js"; +/** + * Valid keys for each action type under the `runs:` section. + * Source: https://github.com/actions/runner/blob/main/src/Runner.Worker/ActionManifestManager.cs + */ +const ACTION_NODE_KEYS = new Set(["using", "main", "pre", "post", "pre-if", "post-if"]); +const ACTION_COMPOSITE_KEYS = new Set(["using", "steps"]); +const ACTION_DOCKER_KEYS = new Set([ + "using", + "image", + "args", + "env", + "entrypoint", + "pre-entrypoint", + "pre-if", + "post-entrypoint", + "post-if" +]); + export function getExpressionInput(input: string, pos: number): string { // Find start marker around the cursor position let startPos = input.lastIndexOf(OPEN_EXPRESSION, pos); @@ -137,7 +155,7 @@ export async function complete( const indentString = " ".repeat(indentation.tabSize); // YAML key/value completions - const values = await getValues( + let values = await getValues( token, keyToken, parent, @@ -147,6 +165,11 @@ export async function complete( schema ); + // Filter action.yml `runs:` completions based on `using:` value + if (isAction && parsedTemplate.value) { + values = filterActionRunsCompletions(values, path, parsedTemplate.value); + } + // Offer "(switch to list)" / "(switch to mapping)" when the schema allows alternative forms const escapeHatches = getEscapeHatchCompletions(token, keyToken, indentString, newPos, schema); values.push(...escapeHatches); @@ -603,3 +626,90 @@ function getOffsetInContent(tokenRange: TokenRange, currentInput: string, pos: P // = 32 + 11 = 43 return lengthOfContentBeforeCurrentLine + pos.character; } + +/** + * Filters action.yml `runs:` completions based on the `using:` value. + * + * When the user is completing keys under `runs:`: + * - If `using: node20` is set, only show Node.js action keys + * - If `using: composite` is set, only show composite action keys + * - If `using: docker` is set, only show Docker action keys + * - If `using:` is not set, show all keys but prioritize `using` first + */ +function filterActionRunsCompletions(values: Value[], path: TemplateToken[], root: TemplateToken): Value[] { + // Find the runs mapping from the root + let runsMapping: MappingToken | undefined; + if (root instanceof MappingToken) { + for (let i = 0; i < root.count; i++) { + const {key, value} = root.get(i); + if (key.toString().toLowerCase() === "runs" && value instanceof MappingToken) { + runsMapping = value; + break; + } + } + } + if (!runsMapping) { + return values; + } + + // Check if the runs mapping is in our path (meaning we're completing inside it) + const isInsideRuns = path.some(token => token === runsMapping); + if (!isInsideRuns) { + return values; + } + + // Also verify we're completing at the runs level, not deeper (like inside steps) + // The runs mapping should be the last mapping in the path before the completion position + // or the path should only have root -> runs + let lastMappingIndex = -1; + for (let i = path.length - 1; i >= 0; i--) { + if (path[i] instanceof MappingToken) { + lastMappingIndex = i; + break; + } + } + if (lastMappingIndex === -1) { + return values; + } + + // If the last mapping in path is not the runs mapping, we're nested deeper (e.g., inside steps) + const lastMapping = path[lastMappingIndex]; + if (lastMapping !== runsMapping) { + return values; + } + + // Get the using value from the runs mapping + let usingValue: string | undefined; + for (let i = 0; i < runsMapping.count; i++) { + const {key, value} = runsMapping.get(i); + if (key.toString().toLowerCase() === "using") { + usingValue = value.toString(); + break; + } + } + + // Determine which keys to allow + let allowedKeys: Set; + + if (!usingValue) { + // No using value set - show all keys but prioritize "using" + return values.map(v => { + if (v.label.toLowerCase() === "using") { + return {...v, sortText: "0_using"}; // Sort first + } + return v; + }); + } else if (usingValue.match(/^node\d+$/i)) { + allowedKeys = ACTION_NODE_KEYS; + } else if (usingValue.toLowerCase() === "composite") { + allowedKeys = ACTION_COMPOSITE_KEYS; + } else if (usingValue.toLowerCase() === "docker") { + allowedKeys = ACTION_DOCKER_KEYS; + } else { + // Unknown using value - show all + return values; + } + + // Filter to only allowed keys + return values.filter(v => allowedKeys.has(v.label.toLowerCase())); +} diff --git a/languageservice/src/validate-action.test.ts b/languageservice/src/validate-action.test.ts index 4050af8..9f7e414 100644 --- a/languageservice/src/validate-action.test.ts +++ b/languageservice/src/validate-action.test.ts @@ -347,4 +347,85 @@ runs: expect(diagnostics).toEqual([]); }); }); + + describe("invalid key combinations based on using type", () => { + it("reports error for node20 action with steps", async () => { + const doc = createActionDocument(` +name: My Action +description: Invalid - node20 with steps +runs: + using: node20 + main: index.js + steps: + - run: echo "hello" + shell: bash +`); + const diagnostics = await validate(doc); + expect(diagnostics.length).toBeGreaterThan(0); + // Schema reports "Unexpected value 'steps'" for invalid keys + expect(diagnostics.some(d => d.message.includes("steps"))).toBe(true); + }); + + it("reports error for composite action with main", async () => { + const doc = createActionDocument(` +name: My Action +description: Invalid - composite with main +runs: + using: composite + steps: + - run: echo "hello" + shell: bash + main: index.js +`); + const diagnostics = await validate(doc); + expect(diagnostics.length).toBeGreaterThan(0); + // Schema reports "Unexpected value 'main'" for invalid keys + expect(diagnostics.some(d => d.message.includes("main"))).toBe(true); + }); + + it("reports error for docker action with steps", async () => { + const doc = createActionDocument(` +name: My Action +description: Invalid - docker with steps +runs: + using: docker + image: Dockerfile + steps: + - run: echo "hello" + shell: bash +`); + const diagnostics = await validate(doc); + expect(diagnostics.length).toBeGreaterThan(0); + // Schema reports "Unexpected value 'steps'" for invalid keys + expect(diagnostics.some(d => d.message.includes("steps"))).toBe(true); + }); + + it("reports error for docker action with main", async () => { + const doc = createActionDocument(` +name: My Action +description: Invalid - docker with main +runs: + using: docker + image: Dockerfile + main: index.js +`); + const diagnostics = await validate(doc); + expect(diagnostics.length).toBeGreaterThan(0); + // Schema reports "Unexpected value 'main'" for invalid keys + expect(diagnostics.some(d => d.message.includes("main"))).toBe(true); + }); + + it("reports error for node20 action missing main", async () => { + const doc = createActionDocument(` +name: My Action +description: Invalid - node20 without main +runs: + using: node20 + pre: setup.js +`); + const diagnostics = await validate(doc); + expect(diagnostics.length).toBeGreaterThan(0); + expect(diagnostics.some(d => d.message.includes("main"))).toBe(true); + }); + }); }); diff --git a/languageservice/src/validate-action.ts b/languageservice/src/validate-action.ts index bc46f22..fce1ba2 100644 --- a/languageservice/src/validate-action.ts +++ b/languageservice/src/validate-action.ts @@ -5,6 +5,7 @@ import {isMapping} from "@actions/workflow-parser"; import {isActionStep} from "@actions/workflow-parser/model/type-guards"; import {ErrorPolicy} from "@actions/workflow-parser/model/convert"; +import {MappingToken} from "@actions/workflow-parser/templates/tokens/mapping-token"; import {SequenceToken} from "@actions/workflow-parser/templates/tokens/sequence-token"; import {TemplateToken} from "@actions/workflow-parser/templates/tokens/template-token"; import {File} from "@actions/workflow-parser/workflows/file"; @@ -16,6 +17,31 @@ import {getOrConvertActionTemplate, getOrParseAction} from "./utils/workflow-cac import {validateActionReference} from "./validate-action-reference.js"; import {ValidationConfig} from "./validate.js"; +/** + * Valid keys for each action type under the `runs:` section. + * Source: https://github.com/actions/runner/blob/main/src/Runner.Worker/ActionManifestManager.cs + */ +const NODE_KEYS = new Set(["using", "main", "pre", "post", "pre-if", "post-if"]); +const COMPOSITE_KEYS = new Set(["using", "steps"]); +const DOCKER_KEYS = new Set([ + "using", + "image", + "args", + "env", + "entrypoint", + "pre-entrypoint", + "pre-if", + "post-entrypoint", + "post-if" +]); + +/** + * Required keys for each action type (besides 'using'). + */ +const NODE_REQUIRED_KEYS = ["main"]; +const COMPOSITE_REQUIRED_KEYS = ["steps"]; +const DOCKER_REQUIRED_KEYS = ["image"]; + /** * Validates an action.yml file * @@ -57,6 +83,12 @@ export async function validateAction(textDocument: TextDocument, config?: Valida }); } + // Validate runs key combinations based on using type + if (result.value) { + const runsKeyDiagnostics = validateRunsKeys(result.value); + diagnostics.push(...runsKeyDiagnostics); + } + // Validate composite action steps if we have a parsed result if (result.value) { const template = getOrConvertActionTemplate(result.context, result.value, textDocument.uri, { @@ -102,3 +134,103 @@ function findStepsSequence(root: TemplateToken): SequenceToken | undefined { } return undefined; } + +/** + * Validates that the keys under `runs:` are valid for the specified `using:` type. + */ +function validateRunsKeys(root: TemplateToken): Diagnostic[] { + const diagnostics: Diagnostic[] = []; + + // Find the runs mapping from the root + let runsMapping: MappingToken | undefined; + if (root instanceof MappingToken) { + for (let i = 0; i < root.count; i++) { + const {key, value} = root.get(i); + if (key.toString().toLowerCase() === "runs" && value instanceof MappingToken) { + runsMapping = value; + break; + } + } + } + if (!runsMapping) { + return diagnostics; + } + + // Get the using value from the runs mapping + let usingValue: string | undefined; + for (let i = 0; i < runsMapping.count; i++) { + const {key, value} = runsMapping.get(i); + if (key.toString().toLowerCase() === "using") { + usingValue = value.toString(); + break; + } + } + if (!usingValue) { + return diagnostics; // No using value, let schema validation handle it + } + + // Determine allowed keys, required keys, and action type name + let allowedKeys: Set; + let requiredKeys: string[]; + let actionType: string; + + if (usingValue.match(/^node\d+$/i)) { + allowedKeys = NODE_KEYS; + requiredKeys = NODE_REQUIRED_KEYS; + actionType = "Node.js"; + } else if (usingValue.toLowerCase() === "composite") { + allowedKeys = COMPOSITE_KEYS; + requiredKeys = COMPOSITE_REQUIRED_KEYS; + actionType = "composite"; + } else if (usingValue.toLowerCase() === "docker") { + allowedKeys = DOCKER_KEYS; + requiredKeys = DOCKER_REQUIRED_KEYS; + actionType = "Docker"; + } else { + return diagnostics; // Unknown type, let schema validation handle it + } + + // Get all present keys + const presentKeys = new Set(); + for (let i = 0; i < runsMapping.count; i++) { + const {key} = runsMapping.get(i); + presentKeys.add(key.toString().toLowerCase()); + } + + // Check for invalid keys + for (let i = 0; i < runsMapping.count; i++) { + const {key} = runsMapping.get(i); + const keyStr = key.toString().toLowerCase(); + + if (!allowedKeys.has(keyStr)) { + diagnostics.push({ + severity: DiagnosticSeverity.Error, + range: mapRange(key.range), + message: `'${key.toString()}' is not valid for ${actionType} actions (using: ${usingValue})` + }); + } + } + + // Check for missing required keys + for (const requiredKey of requiredKeys) { + if (!presentKeys.has(requiredKey)) { + // Find the 'using' key to report the error location + let usingKeyRange = runsMapping.range; + for (let i = 0; i < runsMapping.count; i++) { + const {key} = runsMapping.get(i); + if (key.toString().toLowerCase() === "using") { + usingKeyRange = key.range; + break; + } + } + + diagnostics.push({ + severity: DiagnosticSeverity.Error, + range: mapRange(usingKeyRange), + message: `'${requiredKey}' is required for ${actionType} actions (using: ${usingValue})` + }); + } + } + + return diagnostics; +} diff --git a/workflow-parser/src/action-v1.0.json b/workflow-parser/src/action-v1.0.json index 6cc4c10..65cc4d8 100644 --- a/workflow-parser/src/action-v1.0.json +++ b/workflow-parser/src/action-v1.0.json @@ -267,6 +267,7 @@ }, "main": { "type": "non-empty-string", + "required": true, "description": "The file that contains your action code. The runtime specified in using executes this file.\n\n[Documentation](https://docs.github.com/actions/creating-actions/metadata-syntax-for-github-actions#runsmain)" }, "pre": { From 78231482f5bca0461a2fddb491b48407cd073774 Mon Sep 17 00:00:00 2001 From: eric sciple Date: Wed, 7 Jan 2026 08:42:59 -0600 Subject: [PATCH 2/3] Fix completion and validation issues in action.yml (#290) Follow-up to https://github.com/actions/languageservices/pull/289 ## What this fixes **Autocomplete was broken inside composite action steps.** When you typed inside a step and triggered autocomplete, nothing showed up. Now you correctly get suggestions like run, uses, shell, etc. **Duplicate error messages for missing required fields.** When a required field was missing (like main for Node.js actions), users saw two error messages - one generic schema validation error, and one custom error with a clear explanation. Now they only see the custom one. For example, with using: node24 but no main: - Before: Two errors shown - Schema: "There's not enough info to determine what you meant. Add one of these properties: args, entrypoint, image, main, ..." - Custom: "'main' is required for Node.js actions (using: node24)" - After: Only the custom error is shown --- languageservice/src/complete-action.test.ts | 23 +++++ languageservice/src/complete.ts | 20 ++--- languageservice/src/validate-action.test.ts | 99 +++++++++++++++++++++ languageservice/src/validate-action.ts | 51 +++++++++-- 4 files changed, 170 insertions(+), 23 deletions(-) diff --git a/languageservice/src/complete-action.test.ts b/languageservice/src/complete-action.test.ts index 76340f4..1ffe00b 100644 --- a/languageservice/src/complete-action.test.ts +++ b/languageservice/src/complete-action.test.ts @@ -262,6 +262,29 @@ runs: // It should have a sortText that makes it sort first expect(usingCompletion?.sortText).toBe("0_using"); }); + + it("completes step keys inside composite action steps", async () => { + const [doc, position] = createActionDocument(`name: Test +description: Test +runs: + using: composite + steps: + - run: echo hello + shell: bash + - |`); + const completions = await complete(doc, position); + const labels = completions.map(c => c.label); + + // Should show step keys, not filtered by runs-level logic + expect(labels).toContain("run"); + expect(labels).toContain("uses"); + expect(labels).toContain("shell"); + expect(labels).toContain("id"); + expect(labels).toContain("name"); + expect(labels).toContain("if"); + expect(labels).toContain("env"); + expect(labels).toContain("working-directory"); + }); }); describe("branding completions", () => { diff --git a/languageservice/src/complete.ts b/languageservice/src/complete.ts index 8b05356..b71da41 100644 --- a/languageservice/src/complete.ts +++ b/languageservice/src/complete.ts @@ -658,23 +658,15 @@ function filterActionRunsCompletions(values: Value[], path: TemplateToken[], roo return values; } - // Also verify we're completing at the runs level, not deeper (like inside steps) - // The runs mapping should be the last mapping in the path before the completion position - // or the path should only have root -> runs - let lastMappingIndex = -1; - for (let i = path.length - 1; i >= 0; i--) { - if (path[i] instanceof MappingToken) { - lastMappingIndex = i; - break; - } - } - if (lastMappingIndex === -1) { + // Find where runsMapping is in the path + const runsMappingIndex = path.indexOf(runsMapping); + if (runsMappingIndex === -1) { return values; } - // If the last mapping in path is not the runs mapping, we're nested deeper (e.g., inside steps) - const lastMapping = path[lastMappingIndex]; - if (lastMapping !== runsMapping) { + // Check if there's anything after runsMapping in the path + // If so, we're nested deeper (e.g., inside steps sequence or a step mapping) + if (runsMappingIndex < path.length - 1) { return values; } diff --git a/languageservice/src/validate-action.test.ts b/languageservice/src/validate-action.test.ts index 9f7e414..495fc0b 100644 --- a/languageservice/src/validate-action.test.ts +++ b/languageservice/src/validate-action.test.ts @@ -427,5 +427,104 @@ runs: expect(diagnostics.length).toBeGreaterThan(0); expect(diagnostics.some(d => d.message.includes("main"))).toBe(true); }); + + it("reports error for node24 action missing main", async () => { + const doc = createActionDocument(` +name: My Action +description: Invalid - node24 without main +runs: + using: node24 + pre: setup.js +`); + const diagnostics = await validate(doc); + expect(diagnostics.some(d => d.message === "'main' is required for Node.js actions (using: node24)")).toBe(true); + // Should NOT have duplicate schema error + expect(diagnostics.filter(d => d.message.includes("main")).length).toBe(1); + }); + + it("reports error for node24 action with only using (no narrowing key)", async () => { + const doc = createActionDocument(` +name: My Action +description: Invalid - node24 without main +runs: + using: node24 +`); + const diagnostics = await validate(doc); + expect(diagnostics.some(d => d.message === "'main' is required for Node.js actions (using: node24)")).toBe(true); + // Should NOT have the generic "not enough info" schema error + expect(diagnostics.some(d => d.message.includes("There's not enough info"))).toBe(false); + }); + + it("reports error for composite action missing steps", async () => { + const doc = createActionDocument(` +name: My Action +description: Invalid - composite without steps +runs: + using: composite +`); + const diagnostics = await validate(doc); + expect(diagnostics.some(d => d.message === "'steps' is required for composite actions (using: composite)")).toBe( + true + ); + // Should NOT have duplicate schema error + expect(diagnostics.some(d => d.message.includes("There's not enough info"))).toBe(false); + }); + + it("reports error for docker action missing image", async () => { + const doc = createActionDocument(` +name: My Action +description: Invalid - docker without image +runs: + using: docker +`); + const diagnostics = await validate(doc); + expect(diagnostics.some(d => d.message === "'image' is required for Docker actions (using: docker)")).toBe(true); + // Should NOT have duplicate schema error + expect(diagnostics.some(d => d.message.includes("There's not enough info"))).toBe(false); + }); + + it("reports error for docker action with entrypoint but missing image", async () => { + const doc = createActionDocument(` +name: My Action +description: Invalid - docker without image +runs: + using: docker + entrypoint: /entrypoint.sh +`); + const diagnostics = await validate(doc); + expect(diagnostics.some(d => d.message === "'image' is required for Docker actions (using: docker)")).toBe(true); + // Should NOT have duplicate "Required property is missing: image" schema error + expect(diagnostics.filter(d => d.message.includes("image")).length).toBe(1); + }); + + it("lets schema handle missing using", async () => { + const doc = createActionDocument(` +name: My Action +description: Invalid - no using +runs: + main: index.js +`); + const diagnostics = await validate(doc); + // Should have schema error about not enough info or unexpected value + expect(diagnostics.length).toBeGreaterThan(0); + // Should NOT have custom validation error (can't determine action type) + expect(diagnostics.some(d => d.message.includes("is required for"))).toBe(false); + }); + + it("lets schema handle invalid using value", async () => { + const doc = createActionDocument(` +name: My Action +description: Invalid - bad using value +runs: + using: not-supported + main: index.js +`); + const diagnostics = await validate(doc); + // Should have schema error about unexpected value + expect(diagnostics.length).toBeGreaterThan(0); + // Should NOT have custom validation error (unknown action type) + expect(diagnostics.some(d => d.message.includes("is required for"))).toBe(false); + expect(diagnostics.some(d => d.message.includes("is not valid for"))).toBe(false); + }); }); }); diff --git a/languageservice/src/validate-action.ts b/languageservice/src/validate-action.ts index fce1ba2..4d4b2a1 100644 --- a/languageservice/src/validate-action.ts +++ b/languageservice/src/validate-action.ts @@ -8,6 +8,7 @@ import {ErrorPolicy} from "@actions/workflow-parser/model/convert"; import {MappingToken} from "@actions/workflow-parser/templates/tokens/mapping-token"; import {SequenceToken} from "@actions/workflow-parser/templates/tokens/sequence-token"; import {TemplateToken} from "@actions/workflow-parser/templates/tokens/template-token"; +import {TemplateValidationError} from "@actions/workflow-parser/templates/template-validation-error"; import {File} from "@actions/workflow-parser/workflows/file"; import {TextDocument} from "vscode-languageserver-textdocument"; import {Diagnostic, DiagnosticSeverity} from "vscode-languageserver-types"; @@ -64,8 +65,16 @@ export async function validateAction(textDocument: TextDocument, config?: Valida return []; } - // Map parser errors to diagnostics - for (const err of result.context.errors.getErrors()) { + // Get schema errors + const schemaErrors = result.context.errors.getErrors(); + + // Run custom runs key validation, which also filters redundant schema errors in place + if (result.value) { + diagnostics.push(...validateRunsKeysAndFilterErrors(result.value, schemaErrors)); + } + + // Map remaining schema errors to diagnostics + for (const err of schemaErrors) { const range = mapRange(err.range); // Determine severity based on error type @@ -83,12 +92,6 @@ export async function validateAction(textDocument: TextDocument, config?: Valida }); } - // Validate runs key combinations based on using type - if (result.value) { - const runsKeyDiagnostics = validateRunsKeys(result.value); - diagnostics.push(...runsKeyDiagnostics); - } - // Validate composite action steps if we have a parsed result if (result.value) { const template = getOrConvertActionTemplate(result.context, result.value, textDocument.uri, { @@ -137,8 +140,12 @@ function findStepsSequence(root: TemplateToken): SequenceToken | undefined { /** * Validates that the keys under `runs:` are valid for the specified `using:` type. + * Also filters out schema errors (in place) that this validation replaces with more specific messages. */ -function validateRunsKeys(root: TemplateToken): Diagnostic[] { +function validateRunsKeysAndFilterErrors( + root: TemplateToken, + schemaErrors: TemplateValidationError[] // mutated: redundant errors are removed +): Diagnostic[] { const diagnostics: Diagnostic[] = []; // Find the runs mapping from the root @@ -232,5 +239,31 @@ function validateRunsKeys(root: TemplateToken): Diagnostic[] { } } + // Remove schema errors that we're replacing with more specific messages (mutate in place) + for (let i = schemaErrors.length - 1; i >= 0; i--) { + const err = schemaErrors[i]; + + // Keep errors not at the runs section start + if ( + err.range?.start.line !== runsMapping.range?.start.line || + err.range?.start.column !== runsMapping.range?.start.column + ) { + continue; + } + + // Check if this is an error we're replacing + const isOneOfAmbiguity = err.rawMessage.startsWith("There's not enough info to determine"); + const isRequiredKey = /^Required property is missing: (main|steps|image)$/.test(err.rawMessage); + + if (!isOneOfAmbiguity && !isRequiredKey) { + continue; // Keep errors we're not replacing + } + + // Remove only if we have custom diagnostics for this + if (diagnostics.length > 0) { + schemaErrors.splice(i, 1); + } + } + return diagnostics; } From dbf7752734727b72c8b8dbddccf5610479179c5c Mon Sep 17 00:00:00 2001 From: eric sciple Date: Wed, 7 Jan 2026 08:43:22 -0600 Subject: [PATCH 3/3] Show cron description on hover (#291) Related #286 - When hovering over a cron expression, show the human-readable description instead of empty content. Users who have inlay hints disabled can now still see the cron description. --- languageservice/src/hover.test.ts | 3 +-- languageservice/src/hover.ts | 13 +++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/languageservice/src/hover.test.ts b/languageservice/src/hover.test.ts index ef0908d..b0217e7 100644 --- a/languageservice/src/hover.test.ts +++ b/languageservice/src/hover.test.ts @@ -110,8 +110,7 @@ jobs: `; const result = await hover(...getPositionFromCursor(input)); expect(result).not.toBeUndefined(); - // Cron description is now shown via diagnostics, not hover - expect(result?.contents).toEqual(""); + expect(result?.contents).toEqual("Runs at 0 and 30 minutes past the hour, at 00:00 and 12:00"); }); it("on a cron mapping key", async () => { diff --git a/languageservice/src/hover.ts b/languageservice/src/hover.ts index b8b6481..873abea 100644 --- a/languageservice/src/hover.ts +++ b/languageservice/src/hover.ts @@ -2,6 +2,8 @@ import {data, DescriptionDictionary, Parser} from "@actions/expressions"; import {FunctionDefinition, FunctionInfo} from "@actions/expressions/funcs/info"; import {Lexer} from "@actions/expressions/lexer"; import {parseAction} from "@actions/workflow-parser/actions/action-parser"; +import {isString} from "@actions/workflow-parser"; +import {getCronDescription} from "@actions/workflow-parser/model/converter/cron"; import {ErrorPolicy} from "@actions/workflow-parser/model/convert"; import {splitAllowedContext} from "@actions/workflow-parser/templates/allowed-context"; import {TemplateToken} from "@actions/workflow-parser/templates/tokens/template-token"; @@ -134,6 +136,17 @@ export async function hover(document: TextDocument, position: Position, config?: // Non-expression hover: show the schema description for the YAML key or value info(`Calculating hover for token with definition ${hoverToken.definition.key}`); + // Check for cron expression hover + if (isString(hoverToken) && hoverToken.definition.key === "cron-pattern") { + const cronDescription = getCronDescription(hoverToken.value); + if (cronDescription) { + return { + contents: cronDescription, + range: mapRange(hoverToken.range) + }; + } + } + let description: string; if (!isAction && tokenResult.parent && isReusableWorkflowJobInput(tokenResult)) { // Reusable workflow call: fetch the called workflow's input descriptions