-
Notifications
You must be signed in to change notification settings - Fork 162
Fix custom jobs persisting in final repositories when removed from templates #1830
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: mazhelez <[email protected]>
@copilot documentation needs to be fixed as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PSScriptAnalyzer found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Co-authored-by: mazhelez <[email protected]>
@copilot can you address these potential problems. It's mainly trailing whitespaces. |
Co-authored-by: mazhelez <[email protected]>
Addressed all PSScriptAnalyzer trailing whitespace issues. Removed trailing whitespaces from all PowerShell files in commit f371a41. All tests continue to pass. |
@copilot Can you add a setting to control the behavior? |
…in final repositories Co-authored-by: mazhelez <[email protected]>
Added the Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
# Extract repository reference from templateUrl (e.g., "microsoft/AL-Go-PTE" from "https://github.com/microsoft/AL-Go-PTE@main") | ||
$templateRepoUrl = $repoSettings.templateUrl.Split('@')[0] | ||
$templateRepoReference = $templateRepoUrl.Split('/')[-2..-1] -join '/' | ||
$isFinalRepository = $templateRepoReference -ne $currentRepoReference |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't correct. Final repo is a repo where the template URL doesn't point to microsoft/AL-Go-PTE
or microsoft/AL-Go-AppSource
or microsoft/AL-Go
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, that's also not correct.
I don't think there's a mechanism currently to differentiate between final and template repos.
Maybe the template repos should be marked as template? (as in GitHub template repo).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the final repository detection logic. The logic now correctly identifies final repositories as those where templateUrl doesn't point to the standard AL-Go repositories (microsoft/AL-Go-PTE, microsoft/AL-Go-AppSource, microsoft/AL-Go). Also removed the unused variable and added comprehensive tests. Commit 990ed65.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the repository detection logic to use GitHub's template repository API instead of templateUrl analysis. The new approach checks the is_template
property from the GitHub API to properly distinguish between template and final repositories. This correctly addresses the issue that there was no reliable mechanism to differentiate repository types. Template repositories (marked as GitHub templates) always allow custom jobs, while final repositories respect the allowCustomJobsInEndRepos
setting. Implemented in commit ed35ddc.
Co-authored-by: mazhelez <[email protected]>
Co-authored-by: mazhelez <[email protected]>
# Mock environment and repo settings for final repository | ||
$env:GITHUB_REPOSITORY = "testowner/final-repo" | ||
$env:GITHUB_API_URL = "https://api.github.com" | ||
$repoSettings = @{} |
Check warning
Code scanning / PSScriptAnalyzer
The variable 'repoSettings' is assigned but never used. Warning
# Mock environment and repo settings for template repository | ||
$env:GITHUB_REPOSITORY = "testowner/template-repo" | ||
$env:GITHUB_API_URL = "https://api.github.com" | ||
$repoSettings = @{} |
Check warning
Code scanning / PSScriptAnalyzer
The variable 'repoSettings' is assigned but never used. Warning
I took a separate approach. Thanks for trying! |
Problem
When using custom templates with AL-Go, custom jobs that are removed from the template repository continue to persist in final repositories that use the template. This happens because the
CheckForUpdates
action always applies custom jobs from the destination repository back to updated workflows, regardless of whether the repository is a template or a final repository.Reproduction scenario:
Solution
Implemented repository type detection to distinguish between:
templateUrl
pointing to another repository → Skip applying custom jobs from current repositorytemplateUrl
→ Continue applying custom jobs normallyChanges Made
Modified
Actions/CheckForUpdates/CheckForUpdates.ps1
:templateUrl
points to another repositoryEnhanced test coverage:
CheckForUpdates.Action.Test.ps1
CustomJobRemoval.Test.ps1
with scenarios for both final and template repositoriesValidation
The fix has been thoroughly tested with:
Test results:
This solution follows the suggested approach to "disallow custom jobs on end repos" while preserving the functionality for template repositories.
Fixes #1829.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.