Skip to content

Prune non-schema entries from SharedWorkflowForbiddenFields#37236

Merged
pelikhan merged 2 commits into
mainfrom
copilot/deep-report-clean-up-dead-entries
Jun 6, 2026
Merged

Prune non-schema entries from SharedWorkflowForbiddenFields#37236
pelikhan merged 2 commits into
mainfrom
copilot/deep-report-clean-up-dead-entries

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 6, 2026

SharedWorkflowForbiddenFields contained entries that cannot match real top-level frontmatter keys (command, roles, timeout_minutes), which made shared-workflow restrictions noisier and misleading relative to main_workflow_schema.json. This change tightens the list to schema-real fields only.

  • Schema-aligned forbidden field list

    • Removed dead entries from /pkg/constants/constants.go:
      • command
      • roles
      • timeout_minutes
    • Kept timeout-minutes (the actual top-level schema key).
  • Documentation/comment alignment

    • Updated the category comments above SharedWorkflowForbiddenFields to remove references to deleted keys.
    • Updated the constants README example to reflect the current list shape.
  • Integration test fixture cleanup

    • Removed stale YAML fixture entries in /pkg/workflow/forbidden_fields_import_test.go for the deleted forbidden keys, keeping fixture coverage aligned with the canonical constant.
var SharedWorkflowForbiddenFields = []string{
    "on",
    "concurrency",
    "container",
    "environment",
    "features",
    "github-token",
    "if",
    "name",
    "run-name",
    "runs-on",
    "sandbox",
    "strict",
    "timeout-minutes",
    "tracker-id",
}

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot AI changed the title [WIP] Clean up dead entries in SharedWorkflowForbiddenFields Prune non-schema entries from SharedWorkflowForbiddenFields Jun 6, 2026
Copilot AI requested a review from gh-aw-bot June 6, 2026 02:40
@pelikhan pelikhan marked this pull request as ready for review June 6, 2026 03:32
Copilot AI review requested due to automatic review settings June 6, 2026 03:32
@pelikhan pelikhan merged commit 0969cc4 into main Jun 6, 2026
@pelikhan pelikhan deleted the copilot/deep-report-clean-up-dead-entries branch June 6, 2026 03:32
Copy link
Copy Markdown
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

Note

Copilot was unable to run its full agentic suite in this review.

Updates the shared-workflow import restrictions by allowing a few previously forbidden top-level fields, and aligns docs/tests with the updated forbidden-field list.

Changes:

  • Removed command, roles, and timeout_minutes from SharedWorkflowForbiddenFields.
  • Updated the forbidden-fields import rejection test cases to stop expecting those fields to be rejected.
  • Updated constants documentation to reflect the new forbidden-fields list.
Show a summary per file
File Description
pkg/workflow/forbidden_fields_import_test.go Removes rejection test cases for fields that are no longer forbidden.
pkg/constants/constants.go Updates the canonical forbidden-fields list and its inline documentation.
pkg/constants/README.md Updates the README example to match the revised forbidden-fields list.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (1)

pkg/workflow/forbidden_fields_import_test.go:1

  • The test claims it is using SharedWorkflowForbiddenFields, but the map keys don’t appear to match the constant list: the constant uses environment (not env). This means the test likely isn’t validating rejection for environment at all. Update the test case key/YAML to use environment, or (preferably) derive/validate the test cases against SharedWorkflowForbiddenFields so drift is caught automatically.
//go:build integration
  • Files reviewed: 3/3 changed files
  • Comments generated: 3

Comment on lines 19 to 20
// Use the SharedWorkflowForbiddenFields constant and create YAML examples for each
forbiddenFieldYAML := map[string]string{
Comment on lines 332 to 347
var SharedWorkflowForbiddenFields = []string{
"on", // Trigger field - only for main workflows
"command", // Command for workflow execution
"concurrency", // Concurrency control
"container", // Container configuration
"environment", // Deployment environment
"features", // Feature flags
"github-token", // GitHub token configuration
"if", // Conditional execution
"name", // Workflow name
"roles", // Role requirements
"run-name", // Run display name
"runs-on", // Runner specification
"sandbox", // Sandbox configuration
"strict", // Strict mode
"timeout-minutes", // Timeout in minutes
"timeout_minutes", // Timeout in minutes (underscore variant)
"tracker-id", // Tracker ID
}
// Forbidden fields fall into these categories:
// - Workflow triggers: on (defines it as a main workflow)
// - Workflow execution: command, run-name, runs-on, concurrency, if, timeout-minutes, timeout_minutes
// - Workflow execution: run-name, runs-on, concurrency, if, timeout-minutes
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.

[deep-report] Clean up dead entries in SharedWorkflowForbiddenFields (command, roles, timeout_minutes)

4 participants