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": {