Merge branch 'main' into allanguigou/case

languageservice/src/complete-action.test.ts

@@ -184,6 +184,107 @@ 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");
+    });
+
+    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

@@ -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,82 @@ 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;
+  }
+
+  // Find where runsMapping is in the path
+  const runsMappingIndex = path.indexOf(runsMapping);
+  if (runsMappingIndex === -1) {
+    return values;
+  }
+
+  // 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;
+  }
+
+  // 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<string>;
+
+  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()));
+}

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

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

languageservice/src/validate-action.test.ts

@@ -347,4 +347,184 @@ 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);
+    });
+
+    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

@@ -5,8 +5,10 @@
 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 {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";
@@ -16,6 +18,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
  *
@@ -38,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
@@ -102,3 +137,133 @@ function findStepsSequence(root: TemplateToken): SequenceToken | undefined {
   }
   return 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 validateRunsKeysAndFilterErrors(
+  root: TemplateToken,
+  schemaErrors: TemplateValidationError[] // mutated: redundant errors are removed
+): 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<string>;
+  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<string>();
+  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})`
+      });
+    }
+  }
+
+  // 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;
+}

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