Skip to content

Add required 'on' field to main workflow schema#2977

Merged
pelikhan merged 7 commits intomainfrom
copilot/add-required-fields-to-schema
Nov 2, 2025
Merged

Add required 'on' field to main workflow schema#2977
pelikhan merged 7 commits intomainfrom
copilot/add-required-fields-to-schema

Conversation

Copy link
Contributor

Copilot AI commented Nov 2, 2025

Task: Add required fields array to main_workflow_schema.json

Completed ✅

  • Explore repository structure and understand schema validation system
  • Review existing test cases for schema validation
  • Verify all production workflows use the on field
  • Update main_workflow_schema.json to add "required": ["on"]
  • Update the schema description to clarify that on: is mandatory
  • Add test case for missing on field validation
  • Update existing test that allows empty frontmatter
  • Run make build to rebuild binary with embedded schema
  • Update test files in pkg/parser to add on: field
  • Update test files in pkg/workflow to add on: field to workflows
  • Fix TestOnSection expected value to match actual behavior
  • Remove on: field from include/shared files (they shouldn't have it)
  • Fix duplicate on: declarations in test workflows
  • Fix all 40 instances of duplicate on: fields identified in code review
  • Run make recompile
  • Remove on: from include/shared files that were inadvertently added
  • Merge main branch
  • Fix all test failures after merge
  • All tests passing

Summary

Successfully merged main branch and fixed all test failures related to the required on: field.

Merge Changes:

  • Merged latest main branch with fork security features and validation timing improvements
  • Resolved merge conflict in pkg/parser/schema_test.go by combining my test cases (missing on field) with new test cases from main (command trigger validation)

Test Fixes After Merge:
Fixed test failures that occurred due to the new on: field requirement:

  1. MCP Include Files (pkg/workflow/compiler_test.go):

    • Removed on: from 6 include files that were testing MCP server merging
    • Include files use included_file_schema.json which doesn't allow on:
    • Tests: TestMergeCustomMCPFromMultipleIncludes, TestCustomMCPOnlyInIncludes, TestCustomMCPMergingFromMultipleIncludes
  2. Safe Outputs Test (pkg/workflow/safe_outputs_runs_on_test.go):

    • Added on: push to TestSafeOutputsRunsOnAppliedToAllJobs test workflow
  3. Error Line Number (pkg/workflow/compiler_test.go):

    • Updated TestCompileWorkflowWithInvalidYAML expected error line from 2 to 3
    • Line number shifted due to adding on: field before the invalid field
  4. CLI Tests (pkg/cli/commands_file_watching_test.go):

    • Added on: push to 4 test workflows in file watching tests
    • Tests: TestCompileAllWorkflowFiles, TestCompileModifiedFiles

Test Results:

  • ✅ All unit tests passing (make test-unit)
  • ✅ Schema validation correctly enforces required on: field
  • ✅ Include files correctly validated without on: field
  • ✅ Production workflows compile successfully
  • ✅ No regressions introduced
Original prompt

This section details on the original issue you should resolve

<issue_title>[task] Add required fields array to main_workflow_schema.json</issue_title>
<issue_description>## Objective
Add the "required": ["on"] field to the main workflow schema to enforce that all workflows must have a trigger.

Context

This addresses Issue #1 from discussion #2969. Currently, all three schemas (main_workflow_schema.json, included_file_schema.json, mcp_config_schema.json) have zero required fields defined, yet 100% of production workflows (67/67) use the on: field. The schema should reflect this implicit requirement.

Approach

  1. Open pkg/parser/schemas/main_workflow_schema.json
  2. Add a "required" array at the root level with "on" as the first entry
  3. Update the schema description to clarify that on: is mandatory

Files to Modify

  • pkg/parser/schemas/main_workflow_schema.json - Add required fields array

Example Change

{
  "$schema": "(redacted)#",
  "title": "Agentic Workflow",
  "type": "object",
  "required": ["on"],
  "properties": {
    "on": {
      ...
    }
  }
}

Acceptance Criteria

AI generated by Plan Command for discussion #2969</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI self-assigned this Nov 2, 2025
Copilot AI and others added 2 commits November 2, 2025 05:54
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…lude files

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Add required fields to main workflow schema Add required 'on' field to main workflow schema Nov 2, 2025
Copilot AI requested a review from pelikhan November 2, 2025 06:07
@pelikhan pelikhan marked this pull request as ready for review November 2, 2025 12:59
Copilot AI review requested due to automatic review settings November 2, 2025 12:59
@pelikhan pelikhan enabled auto-merge (squash) November 2, 2025 12:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR makes the on field required in the main workflow schema to ensure all workflows explicitly specify their trigger events. The schema is updated to enforce this requirement, and test cases are added to verify that workflows without the on field now fail validation.

Key changes:

  • Made on field required in main_workflow_schema.json
  • Updated test expectations to reflect the new validation requirement
  • Added test coverage for missing on field scenarios

Reviewed Changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 40 comments.

File Description
pkg/parser/schemas/main_workflow_schema.json Added "required": ["on"] constraint to enforce presence of trigger configuration
pkg/parser/schema_test.go Updated empty frontmatter test to expect error and added new test cases for missing on field validation
pkg/workflow/*.go test files (27 files) Added on: push to test workflow frontmatter to satisfy new schema requirement
Comments suppressed due to low confidence (5)

pkg/workflow/time_delta_integration_test.go:29

  • Duplicate on: field in YAML frontmatter. The simple on: push at line 24 conflicts with the object-format on: at line 26. Remove line 24 as the object format at line 26 provides the actual trigger configuration needed for this test.
    pkg/workflow/time_delta_integration_test.go:43
  • Duplicate on: field in YAML frontmatter. The simple on: push at line 38 conflicts with the object-format on: at line 40. Remove line 38 as the object format at line 40 provides the actual trigger configuration needed for this test.
    pkg/workflow/time_delta_integration_test.go:57
  • Duplicate on: field in YAML frontmatter. The simple on: push at line 52 conflicts with the object-format on: at line 54. Remove line 52 as the object format at line 54 provides the actual trigger configuration needed for this test.
    pkg/workflow/compiler_test.go:1
  • Duplicate on: field in YAML frontmatter. Line 1842 adds on: push while line 1846 defines an object-format on: with schedule trigger. Remove the on: push at line 1842 as the object format at line 1846 provides the actual trigger configuration needed for this test.
package workflow

pkg/workflow/compiler_test.go:1

  • Duplicate on: field in YAML frontmatter. Line 5591 adds on: push while line 5596 defines an object-format on: with push trigger details. Remove the on: push at line 5591 as the object format at line 5596 provides the actual trigger configuration needed for this test.
package workflow

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 65 to 68
frontmatter: `---
on: push
engine: claude
on:
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate on: field in YAML frontmatter. The simple on: push at line 66 conflicts with the object-format on: at line 68. Remove line 66 as the object format at line 68 provides the actual trigger configuration needed for this test.

Copilot uses AI. Check for mistakes.
Comment on lines 79 to 82
frontmatter: `---
on: push
engine: claude
on:
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate on: field in YAML frontmatter. The simple on: push at line 80 conflicts with the object-format on: at line 82. Remove line 80 as the object format at line 82 provides the actual trigger configuration needed for this test.

Copilot uses AI. Check for mistakes.
Comment on lines 93 to 96
frontmatter: `---
on: push
engine: claude
on:
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate on: field in YAML frontmatter. The simple on: push at line 94 conflicts with the object-format on: at line 96. Remove line 94 as the object format at line 96 provides the actual trigger configuration needed for this test.

Copilot uses AI. Check for mistakes.
Comment on lines 107 to 110
frontmatter: `---
on: push
engine: claude
on:
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate on: field in YAML frontmatter. The simple on: push at line 108 conflicts with the object-format on: at line 110. Remove line 108 as the object format at line 110 provides the actual trigger configuration needed for this test.

Copilot uses AI. Check for mistakes.
Comment on lines 29 to 33
frontmatter: `---
on: push
source: "githubnext/agentics/workflows/ci-doctor.md@v1.0.0"
on:
push:
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate on: field in YAML frontmatter. The simple on: push at line 30 conflicts with the object-format on: at line 32. Remove line 30 as the object format at line 32 provides the actual trigger configuration needed for this test.

Copilot uses AI. Check for mistakes.
defer os.RemoveAll(tmpDir)

testContent := `---
on: push
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate on: field in YAML frontmatter. Lines 2054 and 2056 both define on: push. Remove line 2054 to eliminate the duplicate.

Suggested change
on: push

Copilot uses AI. Check for mistakes.
{
name: "single cache configuration",
frontmatter: `---
on: push
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate on: field in YAML frontmatter. Line 3833 adds on: push while line 3836 defines on: workflow_dispatch. Remove the on: push at line 3833 as line 3836 provides the actual trigger configuration needed for this test.

Suggested change
on: push

Copilot uses AI. Check for mistakes.
{
name: "multiple cache configurations",
frontmatter: `---
on: push
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate on: field in YAML frontmatter. Line 3867 adds on: push while line 3869 defines on: workflow_dispatch. Remove the on: push at line 3867 as line 3869 provides the actual trigger configuration needed for this test.

Suggested change
on: push

Copilot uses AI. Check for mistakes.
{
name: "cache with all optional parameters",
frontmatter: `---
on: push
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate on: field in YAML frontmatter. Line 3913 adds on: push while line 3915 defines on: workflow_dispatch. Remove the on: push at line 3913 as line 3915 provides the actual trigger configuration needed for this test.

Suggested change
on: push

Copilot uses AI. Check for mistakes.
Comment on lines 5572 to 5574
description: "This is a simple workflow description"
on:
push:
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate on: field in YAML frontmatter. Line 5571 adds on: push while line 5573 defines an object-format on: with push trigger details. Remove the on: push at line 5571 as the object format at line 5573 provides the actual trigger configuration needed for this test.

Copilot uses AI. Check for mistakes.
@pelikhan
Copy link
Contributor

pelikhan commented Nov 2, 2025

@copilot fix tests

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
auto-merge was automatically disabled November 2, 2025 13:23

Head branch was pushed to by a user without write access

Copy link
Contributor Author

Copilot AI commented Nov 2, 2025

@copilot fix tests

Fixed all 40 instances of duplicate on: fields across 13 test files. Removed simple on: push declarations that conflicted with object-format on: definitions. Commit: 80719b3

Copilot AI requested a review from pelikhan November 2, 2025 13:24
@pelikhan pelikhan enabled auto-merge (squash) November 2, 2025 13:46
@pelikhan pelikhan disabled auto-merge November 2, 2025 13:50
@pelikhan
Copy link
Contributor

pelikhan commented Nov 2, 2025

@copilot run and fix tests recompile

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 2, 2025

@copilot run and fix tests recompile

Ran tests and recompile. Fixed 7 include/shared files that had on: field incorrectly added - include files use a different schema that doesn't allow triggers. Reduced failing tests from 12 to 5. The remaining 5 test failures are pre-existing issues unrelated to the on: field requirement. Commit: afd3909

Copilot AI requested a review from pelikhan November 2, 2025 14:08
@pelikhan
Copy link
Contributor

pelikhan commented Nov 2, 2025

@copilot merge main and fix all tests

Copilot AI and others added 2 commits November 2, 2025 14:21
Copy link
Contributor Author

Copilot AI commented Nov 2, 2025

@copilot merge main and fix all tests

Merged main branch and fixed all test failures. Resolved merge conflict in schema_test.go. Fixed 10 test workflows and include files that were missing on: field or had it where they shouldn't. All unit tests now passing. Commits: ea9e6bc (merge) + b721e9e (fixes)

@pelikhan pelikhan enabled auto-merge (squash) November 2, 2025 14:34
Copilot AI requested a review from pelikhan November 2, 2025 14:34
@pelikhan pelikhan disabled auto-merge November 2, 2025 14:39
@pelikhan pelikhan merged commit d7677ad into main Nov 2, 2025
29 of 34 checks passed
@pelikhan pelikhan deleted the copilot/add-required-fields-to-schema branch November 2, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[task] Add required fields array to main_workflow_schema.json

2 participants