Add timezone to workflow and pass FF (#334)

* Add timezone to workflow and pass FF

* Prettier fixes

* Prettier fixes

* Prettier fixes

* Guard timezone autocomplete behind FF

* Prettier fix

* Address PR comments

* Prettier fix

* Remove comma

* Remove template assignment

* Move description

* Fix test

* Prettier again!

* Address comments

* Change error when timezone key is entered but FF is off

* Prettier

---------

Co-authored-by: Angel Kou <jiakou@microsoft.com>

Author: angel-jiakou

expressions/src/features.test.ts

@@ -54,7 +54,8 @@ describe("FeatureFlags", () => {
       expect(flags.getEnabledFeatures()).toEqual([
         "missingInputsQuickfix",
         "blockScalarChompingWarning",
-        "allowCaseFunction"
+        "allowCaseFunction",
+        "allowCronTimezone"
       ]);
     });
   });

expressions/src/features.ts

@@ -34,6 +34,12 @@ export interface ExperimentalFeatures {
    * @default false
    */
   allowCaseFunction?: boolean;
+
+  /**
+   * Enable the timezone input in cron schedule mappings.
+   * @default false
+   */
+  allowCronTimezone?: boolean;
 }
 
 /**
@@ -48,7 +54,8 @@ export type ExperimentalFeatureKey = Exclude<keyof ExperimentalFeatures, "all">;
 const allFeatureKeys: ExperimentalFeatureKey[] = [
   "missingInputsQuickfix",
   "blockScalarChompingWarning",
-  "allowCaseFunction"
+  "allowCaseFunction",
+  "allowCronTimezone"
 ];
 
 export class FeatureFlags {

languageservice/src/complete.test.ts

@@ -925,3 +925,45 @@ jobs:
     });
   });
 });
+
+describe("schedule timezone completion", () => {
+  it("includes timezone when allowCronTimezone is enabled", async () => {
+    const input = `on:
+  schedule:
+    - |`;
+    const result = await complete(...getPositionFromCursor(input), {
+      featureFlags: new FeatureFlags({allowCronTimezone: true})
+    });
+
+    expect(result).not.toBeUndefined();
+    const labels = result.map(x => x.label);
+    expect(labels).toContain("cron");
+    expect(labels).toContain("timezone");
+  });
+
+  it("excludes timezone when allowCronTimezone is disabled", async () => {
+    const input = `on:
+  schedule:
+    - |`;
+    const result = await complete(...getPositionFromCursor(input), {
+      featureFlags: new FeatureFlags({allowCronTimezone: false})
+    });
+
+    expect(result).not.toBeUndefined();
+    const labels = result.map(x => x.label);
+    expect(labels).toContain("cron");
+    expect(labels).not.toContain("timezone");
+  });
+
+  it("excludes timezone when no feature flags are provided", async () => {
+    const input = `on:
+  schedule:
+    - |`;
+    const result = await complete(...getPositionFromCursor(input));
+
+    expect(result).not.toBeUndefined();
+    const labels = result.map(x => x.label);
+    expect(labels).toContain("cron");
+    expect(labels).not.toContain("timezone");
+  });
+});

languageservice/src/complete.ts

@@ -163,6 +163,11 @@ export async function complete(
     values = filterActionRunsCompletions(values, path, parsedTemplate.value);
   }
 
+  // Filter `timezone` from schedule completions when the feature flag is disabled
+  if (!config?.featureFlags?.isEnabled("allowCronTimezone") && parent?.definition?.key === "schedule") {
+    values = values.filter(v => v.label !== "timezone");
+  }
+
   // Offer "(switch to list)" / "(switch to mapping)" when the schema allows alternative forms
   const escapeHatches = getEscapeHatchCompletions(token, keyToken, indentString, newPos, schema);
   values.push(...escapeHatches);

languageservice/src/hover.test.ts

@@ -120,7 +120,9 @@ jobs:
 `;
     const result = await hover(...getPositionFromCursor(input));
     expect(result).not.toBeUndefined();
-    expect(result?.contents).toEqual("");
+    expect(result?.contents).toEqual(
+      "A cron expression that represents a schedule. A scheduled workflow will run at most once every 5 minutes."
+    );
   });
 
   it("on an invalid cron schedule", async () => {
@@ -130,7 +132,9 @@ jobs:
 `;
     const result = await hover(...getPositionFromCursor(input));
     expect(result).not.toBeUndefined();
-    expect(result?.contents).toEqual("");
+    expect(result?.contents).toEqual(
+      "A cron expression that represents a schedule. A scheduled workflow will run at most once every 5 minutes."
+    );
   });
 
   it("shows context inherited from parent nodes", async () => {

languageservice/src/validate.ts

@@ -84,7 +84,8 @@ async function validateWorkflow(textDocument: TextDocument, config?: ValidationC
       // Errors will be updated in the context. Attempt to do the conversion anyway in order to give the user more information
       const template = await getOrConvertWorkflowTemplate(result.context, result.value, textDocument.uri, config, {
         fetchReusableWorkflowDepth: config?.fileProvider ? 1 : 0,
-        errorPolicy: ErrorPolicy.TryConversion
+        errorPolicy: ErrorPolicy.TryConversion,
+        featureFlags: config?.featureFlags
       });
 
       // Validate expressions and value providers

workflow-parser/src/model/convert.test.ts

@@ -1,4 +1,5 @@
 /* eslint-disable @typescript-eslint/no-non-null-assertion */
+import {FeatureFlags} from "@actions/expressions/features";
 import {nullTrace} from "../test-utils/null-trace.js";
 import {parseWorkflow} from "../workflows/workflow-parser.js";
 import {convertWorkflowTemplate, ErrorPolicy} from "./convert.js";
@@ -578,4 +579,140 @@ jobs:
       }
     });
   });
+
+  describe("schedule timezone with feature flags", () => {
+    it("allows timezone when allowCronTimezone is enabled", async () => {
+      const result = parseWorkflow(
+        {
+          name: "wf.yaml",
+          content: `on:
+  schedule:
+    - cron: '0 0 * * *'
+      timezone: America/New_York
+jobs:
+  build:
+    runs-on: ubuntu-latest`
+        },
+        nullTrace
+      );
+
+      const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
+        errorPolicy: ErrorPolicy.TryConversion,
+        featureFlags: new FeatureFlags({allowCronTimezone: true})
+      });
+
+      expect(result.context.errors.getErrors()).toHaveLength(0);
+      expect(template.events?.schedule).toHaveLength(1);
+      expect(template.events?.schedule?.[0]).toEqual({
+        cron: "0 0 * * *",
+        timezone: "America/New_York"
+      });
+    });
+
+    it("reports error when timezone is present but allowCronTimezone is disabled", async () => {
+      const result = parseWorkflow(
+        {
+          name: "wf.yaml",
+          content: `on:
+  schedule:
+    - cron: '0 0 * * *'
+      timezone: America/New_York
+jobs:
+  build:
+    runs-on: ubuntu-latest`
+        },
+        nullTrace
+      );
+
+      const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
+        errorPolicy: ErrorPolicy.TryConversion,
+        featureFlags: new FeatureFlags({allowCronTimezone: false})
+      });
+
+      // When timezone feature is disabled, error points at the timezone key
+      expect(result.context.errors.getErrors()).toHaveLength(1);
+      expect(result.context.errors.getErrors()[0].message).toContain("Key 'timezone' is not supported");
+      // Schedule entry is dropped due to unsupported key
+      expect(template.events?.schedule).toHaveLength(0);
+    });
+
+    it("reports error when timezone is present with no feature flags provided", async () => {
+      const result = parseWorkflow(
+        {
+          name: "wf.yaml",
+          content: `on:
+  schedule:
+    - cron: '0 0 * * *'
+      timezone: America/New_York
+jobs:
+  build:
+    runs-on: ubuntu-latest`
+        },
+        nullTrace
+      );
+
+      await convertWorkflowTemplate(result.context, result.value!, undefined, {
+        errorPolicy: ErrorPolicy.TryConversion
+      });
+
+      // Default is timezone disabled, so error points at the timezone key
+      expect(result.context.errors.getErrors()).toHaveLength(1);
+      expect(result.context.errors.getErrors()[0].message).toContain("Key 'timezone' is not supported");
+    });
+
+    it("reports error when cron is missing from schedule entry", async () => {
+      const result = parseWorkflow(
+        {
+          name: "wf.yaml",
+          content: `on:
+  schedule:
+    - timezone: America/New_York
+jobs:
+  build:
+    runs-on: ubuntu-latest`
+        },
+        nullTrace
+      );
+
+      const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
+        errorPolicy: ErrorPolicy.TryConversion,
+        featureFlags: new FeatureFlags({allowCronTimezone: true})
+      });
+
+      // Both schema validation and converter report the missing cron
+      expect(result.context.errors.getErrors().length).toBeGreaterThanOrEqual(1);
+      const errorMessages = result.context.errors
+        .getErrors()
+        .map(e => e.message)
+        .join(", ");
+      expect(errorMessages).toMatch(/Required property is missing: cron|Missing required key 'cron'/);
+      expect(template.events?.schedule).toHaveLength(0);
+    });
+
+    it("converts schedule without timezone when allowCronTimezone is enabled", async () => {
+      const result = parseWorkflow(
+        {
+          name: "wf.yaml",
+          content: `on:
+  schedule:
+    - cron: '0 0 * * *'
+jobs:
+  build:
+    runs-on: ubuntu-latest`
+        },
+        nullTrace
+      );
+
+      const template = await convertWorkflowTemplate(result.context, result.value!, undefined, {
+        errorPolicy: ErrorPolicy.TryConversion,
+        featureFlags: new FeatureFlags({allowCronTimezone: true})
+      });
+
+      expect(result.context.errors.getErrors()).toHaveLength(0);
+      expect(template.events?.schedule).toHaveLength(1);
+      expect(template.events?.schedule?.[0]).toEqual({
+        cron: "0 0 * * *"
+      });
+    });
+  });
 });

workflow-parser/src/model/convert.ts

@@ -1,3 +1,4 @@
+import {FeatureFlags} from "@actions/expressions/features";
 import {TemplateContext} from "../templates/template-context.js";
 import {TemplateToken, TemplateTokenError} from "../templates/tokens/template-token.js";
 import {FileProvider} from "../workflows/file-provider.js";
@@ -37,12 +38,18 @@ export type WorkflowTemplateConverterOptions = {
    * By default, conversion will be skipped if there are errors in the {@link TemplateContext}.
    */
   errorPolicy?: ErrorPolicy;
+
+  /**
+   * Optional feature flags to control which experimental features are enabled.
+   */
+  featureFlags?: FeatureFlags;
 };
 
 const defaultOptions: Required<WorkflowTemplateConverterOptions> = {
   maxReusableWorkflowDepth: 4,
   fetchReusableWorkflowDepth: 0,
-  errorPolicy: ErrorPolicy.ReturnErrorsOnly
+  errorPolicy: ErrorPolicy.ReturnErrorsOnly,
+  featureFlags: new FeatureFlags()
 };
 
 export async function convertWorkflowTemplate(
@@ -54,6 +61,11 @@ export async function convertWorkflowTemplate(
   const result = {} as WorkflowTemplate;
   const opts = getOptionsWithDefaults(options);
 
+  // Store feature flags in context state so converters can access them
+  if (opts.featureFlags) {
+    context.state["featureFlags"] = opts.featureFlags;
+  }
+
   if (context.errors.getErrors().length > 0 && opts.errorPolicy === ErrorPolicy.ReturnErrorsOnly) {
     result.errors = context.errors.getErrors().map(x => ({
       Message: x.message
@@ -142,6 +154,7 @@ function getOptionsWithDefaults(options: WorkflowTemplateConverterOptions): Requ
       options.fetchReusableWorkflowDepth !== undefined
         ? options.fetchReusableWorkflowDepth
         : defaultOptions.fetchReusableWorkflowDepth,
-    errorPolicy: options.errorPolicy !== undefined ? options.errorPolicy : defaultOptions.errorPolicy
+    errorPolicy: options.errorPolicy !== undefined ? options.errorPolicy : defaultOptions.errorPolicy,
+    featureFlags: options.featureFlags ?? defaultOptions.featureFlags
   };
 }

workflow-parser/src/model/converter/events.ts

@@ -1,3 +1,4 @@
+import {FeatureFlags} from "@actions/expressions/features";
 import {TemplateContext} from "../../templates/template-context.js";
 import {MappingToken} from "../../templates/tokens/mapping-token.js";
 import {SequenceToken} from "../../templates/tokens/sequence-token.js";
@@ -55,7 +56,8 @@ export function convertOn(context: TemplateContext, token: TemplateToken): Event
       // Schedule is the only event that can be a sequence, handle that separately
       if (eventName === "schedule") {
         const scheduleToken = item.value.assertSequence(`event ${eventName}`);
-        result.schedule = convertSchedule(context, scheduleToken);
+        const featureFlags = context.state["featureFlags"] as FeatureFlags | undefined;
+        result.schedule = convertSchedule(context, scheduleToken, featureFlags);
         continue;
       }
 
@@ -147,25 +149,47 @@ function convertFilter<T extends TypesFilterConfig & WorkflowFilterConfig & Vers
   return result;
 }
 
-function convertSchedule(context: TemplateContext, token: SequenceToken): ScheduleConfig[] | undefined {
+function convertSchedule(
+  context: TemplateContext,
+  token: SequenceToken,
+  featureFlags?: FeatureFlags
+): ScheduleConfig[] | undefined {
+  const flags = featureFlags ?? new FeatureFlags();
+  const allowTimezone = flags.isEnabled("allowCronTimezone");
   const result = [] as ScheduleConfig[];
+
   for (const item of token) {
     const mappingToken = item.assertMapping(`event schedule`);
-    if (mappingToken.count == 1) {
-      const schedule = mappingToken.get(0);
-      const scheduleKey = schedule.key.assertString(`schedule key`);
-      if (scheduleKey.value == "cron") {
-        const cron = schedule.value.assertString(`schedule cron`);
-        // Validate the cron string
+    const config: ScheduleConfig = {cron: ""};
+    let valid = true;
+
+    for (const entry of mappingToken) {
+      const key = entry.key.assertString(`schedule key`);
+
+      if (key.value === "cron") {
+        const cron = entry.value.assertString(`schedule cron`);
         if (!isValidCron(cron.value)) {
           context.error(cron, "Invalid cron expression. Expected format: '* * * * *' (minute hour day month weekday)");
         }
-        result.push({cron: cron.value});
+        config.cron = cron.value;
+      } else if (key.value === "timezone") {
+        if (allowTimezone) {
+          const timezone = entry.value.assertString(`schedule timezone`);
+          config.timezone = timezone.value;
+        } else {
+          context.error(key, `Key 'timezone' is not supported`);
+          valid = false;
+        }
       } else {
-        context.error(scheduleKey, `Invalid schedule key`);
+        context.error(key, `Invalid schedule key`);
+        valid = false;
       }
-    } else {
-      context.error(mappingToken, "Invalid format for 'schedule'");
+    }
+
+    if (valid && config.cron) {
+      result.push(config);
+    } else if (valid && !config.cron) {
+      context.error(mappingToken, "Missing required key 'cron' in schedule entry");
     }
   }
 

workflow-parser/src/model/workflow-template.ts

@@ -196,6 +196,7 @@ export type SecretConfig = {
 
 export type ScheduleConfig = {
   cron: string;
+  timezone?: string;
 };
 
 export type WorkflowFilterConfig = {