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

@@ -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", () => {

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;
   }
 

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);
+    });
   });
 });

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;
 }