Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 9, 2025

The old top-level safe-jobs: syntax is already rejected by JSON schema validation, but comments incorrectly claimed backwards compatibility existed. No code actually reads from the old location.

Changes

  • pkg/workflow/compiler.go: Removed claim of "old location (safe-jobs) for backwards compatibility"
  • pkg/workflow/safe_jobs.go: Clarified parseSafeJobsConfig() and extractSafeJobsFromFrontmatter() are internal helpers that never read user's top-level safe-jobs: key
  • pkg/workflow/safe_jobs_test.go: Added context that tests validate internal parsing, not user frontmatter
  • pkg/workflow/safe_jobs_syntax_test.go: Added explicit tests verifying schema rejection of old syntax

Current Behavior

Old syntax is rejected with helpful error:

# ❌ Rejected by schema
safe-jobs:
  deploy:
    runs-on: ubuntu-latest

# Error: Unknown property: safe-jobs. Did you mean: safe-outputs

New syntax compiles correctly:

# ✅ Accepted
safe-outputs:
  jobs:
    deploy:
      runs-on: ubuntu-latest

All 74 repository workflows continue to compile successfully.

Original prompt

This section details on the original issue you should resolve

<issue_title>[task] Add deprecation warning for safe-jobs legacy syntax</issue_title>
<issue_description>## Objective
Add deprecation warning when the old safe-jobs syntax is detected, guiding users to migrate to the new safe-outputs.jobs location.

Context

Migration to safe-outputs is 100% complete (0/74 workflows use old syntax), but backwards compatibility code still silently supports the old location. Users should be notified when using deprecated syntax.

Related to #3511

Approach

  1. Detect when workflow uses old safe-jobs: location
  2. Emit warning message to stderr using console.FormatWarningMessage()
  3. Continue processing (maintain backwards compatibility)
  4. Include migration guidance in warning message

Files to Modify

  • Update: pkg/workflow/safe_jobs.go
    • Add deprecation check in parseSafeJobsConfig() function
    • Emit warning when old syntax detected
  • Update: pkg/workflow/compiler.go
    • Add warning when both old and new syntax detected
  • Update: Documentation
    • Add migration guide to troubleshooting section
    • Update examples to show recommended syntax

Warning Message Format

Warning: 'safe-jobs' configuration is deprecated and will be removed in v2.0.0
Please migrate to 'safe-outputs' syntax. See migration guide: [URL]

Acceptance Criteria

  • Warning emitted when safe-jobs: detected in frontmatter
  • Warning includes clear migration instructions
  • Warning references documentation URL for migration guide
  • Backwards compatibility maintained (no breaking changes)
  • Test case added for deprecation warning
  • Documentation updated with migration guide
  • Consider adding migration timeline (removal in next major version)
    Related to [Schema Consistency] 🔍 Schema Consistency Check - November 9, 2025 #3511

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

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

@pelikhan @copilot remove old syntax supper entirely

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits November 9, 2025 06:01
- Update misleading comment in compiler.go that claimed old syntax was supported
- Clarify that parseSafeJobsConfig() is an internal helper function
- Update extractSafeJobsFromFrontmatter() comment to explicitly state old syntax is NOT supported
- Add clarifying comments in test to explain it's testing internal parsing logic
- Old syntax (top-level safe-jobs:) is already rejected by JSON schema validation

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- TestOldSafeJobsSyntaxRejected: Verifies old safe-jobs syntax is rejected by schema
- TestNewSafeOutputsJobsSyntaxAccepted: Verifies new safe-outputs.jobs syntax works
- Tests confirm JSON schema properly enforces the correct syntax

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Add deprecation warning for safe-jobs legacy syntax Remove obsolete safe-jobs backwards compatibility references Nov 9, 2025
Copilot AI requested a review from pelikhan November 9, 2025 06:07
Copilot finished work on behalf of pelikhan November 9, 2025 06:07
@pelikhan pelikhan marked this pull request as ready for review November 9, 2025 07:20
Copilot AI review requested due to automatic review settings November 9, 2025 07:20
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 updates documentation and comments to clarify that the old top-level safe-jobs syntax is no longer supported, and users must use the new safe-outputs.jobs syntax instead. The schema validation already enforces this restriction since the main workflow schema has "additionalProperties": false and does not include safe-jobs as a valid property.

Key Changes:

  • Added comprehensive tests to verify the old safe-jobs syntax is rejected and the new safe-outputs.jobs syntax works correctly
  • Updated comments throughout the codebase to explicitly document that parseSafeJobsConfig is an internal helper and that user workflows must use safe-outputs.jobs

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
pkg/workflow/safe_jobs_test.go Updated comment to clarify that the test validates internal parsing logic, not user-facing syntax
pkg/workflow/safe_jobs_syntax_test.go Added new test file with two tests: one verifying old syntax rejection and one verifying new syntax acceptance
pkg/workflow/safe_jobs.go Enhanced comments on parseSafeJobsConfig and extractSafeJobsFromFrontmatter to explicitly state that top-level safe-jobs is not supported
pkg/workflow/compiler.go Updated comment to remove backward compatibility reference and clarify only safe-outputs.jobs location is checked

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

@pelikhan pelikhan merged commit 9952616 into main Nov 9, 2025
91 of 95 checks passed
@pelikhan pelikhan deleted the copilot/add-deprecation-warning-safe-jobs branch November 9, 2025 18:45
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 deprecation warning for safe-jobs legacy syntax

2 participants