Enhance customALGoFiles feature - #2273
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR enhances the customALGoFiles behavior during “Update AL-Go System Files”, adding support for unconditional removals (filesToRemove) and improving custom-template behavior by merging template settings directly and resolving files from the original AL-Go template where applicable.
Changes:
- Add
customALGoFiles.filesToRemovesupport end-to-end (schema, defaults, resolution logic, docs, and release notes). - Update
CheckForUpdatesto read template repo settings viaReadSettingsand merge template settings during file resolution. - Expand automated coverage (unit + e2e) for include/exclude/remove resolution and custom-template propagation.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| e2eTests/scenarios/CustomTemplate/runtest.ps1 | Extends e2e scenario to validate custom-template file include/exclude/remove propagation and workflow presence. |
| Tests/CheckForUpdates.Action.Test.ps1 | Adds unit tests for destination-folder resolution and expanded GetFilesToUpdate behaviors (including filesToRemove). |
| Actions/CheckForUpdates/CheckForUpdates.ps1 | Updates settings reading (incl. trigger) and wires template settings + filesToRemove into update/removal flow. |
| Actions/CheckForUpdates/CheckForUpdates.HelperFunctions.ps1 | Implements ResolveFilePathsInDestinationFolder and extends GetFilesToUpdate to produce include/exclude/remove lists. |
| Actions/.Modules/settings.schema.json | Extends settings schema with customALGoFiles.filesToRemove and clarifying descriptions. |
| Actions/.Modules/ReadSettings.psm1 | Adds default filesToRemove array under customALGoFiles. |
| Scenarios/settings.md | Documents customALGoFiles.filesToRemove in settings reference. |
| Scenarios/CustomizingALGoForGitHub.md | Adds conceptual docs + examples for original-template resolution and filesToRemove. |
| RELEASENOTES.md | Documents enhanced customALGoFiles behavior and new filesToRemove. |
|
@mazhelez The failing PS5 tests should be fixed now. |
| } | ||
| }, | ||
| "filesToRemove": { | ||
| "description": "An array of file specifications to unconditionally remove from the repository during 'Update AL-Go System Files', regardless of whether the files exist in the template. Useful for cleaning up files that have been removed from the template but may still exist in repositories.", |
There was a problem hiding this comment.
... regardless of whether the files exist in the template
That's the part that made us leave out filesToRemove from the initial implementation of "custom AL-Go files".
If the files are not in the template, they are not considered AL-Go files, so why would AL-Go clean them up?
There was a problem hiding this comment.
Right now a custom template (and AL-Go) cannot remove/rename a file, because these changes in the template are not propagated to repos that use it.
The current workaround is to keep the file in the template and add it to "filesToExclude". This works, but it’s messy and pollutes the template repository.
With filesToRemove a custom template could remove files in the final repositories without the need to keep them.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
Actions/CheckForUpdates/CheckForUpdates.HelperFunctions.ps1:1205
- Removal destinations are not constrained to
$baseFolder. AfilesToRemoveentry with a matching template file anddestinationFolder = '..'produces adestinationFullPathoutside the repository;CheckForUpdates.ps1later converts it to a relative path and passes it toRemove-Item. Normalize every removal destination and reject paths that are not descendants of the repository root before returning this list.
$filesToRemove += @(ResolveFilePaths -sourceFolder $originalTemplateFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
}
$filesToRemove += @(ResolveFilePaths -sourceFolder $templateFolder -originalSourceFolder $originalTemplateFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
$filesToRemove += @(ResolveFilePathsInDestinationFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
| if ($null -ne $templateSettings) { | ||
| $filesToIncludeUnresolved += $templateSettings.customALGoFiles.filesToInclude | ||
| } | ||
| $filesToIncludeUnresolved += $settings.customALGoFiles.filesToInclude |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
Actions/CheckForUpdates/CheckForUpdates.HelperFunctions.ps1:1205
- The source-path boundary check does not validate the generated destination. A spec can match a legitimate template file while setting
destinationFolderto../..., producing afilesToRemoveentry outside$baseFolder;CheckForUpdates.ps1later converts that to a relative traversal path and passes it toRemove-Item. Normalize eachdestinationFullPathand reject entries outside the repository root before returning this removal list.
$filesToRemove += @(ResolveFilePaths -sourceFolder $originalTemplateFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
}
$filesToRemove += @(ResolveFilePaths -sourceFolder $templateFolder -originalSourceFolder $originalTemplateFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
$filesToRemove += @(ResolveFilePathsInDestinationFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
| (Join-Path (Get-Location) $defaultCustomFileName) | Should -Not -Exist | ||
| # Check that optional custom file is NOT present in final repository | ||
| (Join-Path (Get-Location) $optionalCustomFileName) | Should -Not -Exist | ||
| # Check that legacy workflow file is present in final repository |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (3)
Actions/CheckForUpdates/CheckForUpdates.HelperFunctions.ps1:1178
- The stated precedence is not applied within a source folder.
ResolveFilePathssuppresses duplicatedestinationFullPathvalues as it iterates, and this unresolved list is ordered defaults → template settings → repository settings, so the first/default or template entry wins before this grouping can keep the last entry. A repository include targeting the same destination as a template include is therefore silently ignored. Resolve all candidates before deduplicating, or process the specifications in precedence order.
# Remove duplicates based on destinationFullPath, keeping the last one (setting > template settings > defaults; template folder > original template folder)
$filesToInclude = @($filesToInclude | Group-Object { $_.destinationFullPath } | Sort-Object -Property Name | ForEach-Object { $_.Group[-1] })
Actions/CheckForUpdates/CheckForUpdates.HelperFunctions.ps1:1205
- Template-resolved removals can produce a
destinationFullPathoutside$baseFolder: for example, an entry withdestinationFolder = '..'is resolved from a valid template source and retained here, even though destination-folder resolution rejects the escaped target. The caller later converts this path to../...and passes it toRemove-Itemin the cloned workspace, enabling deletion attempts outside the repository or causing the update to fail. Normalize each destination withGetFullPathand reject paths that are not strictly within$baseFolderbefore returning removal entries.
$filesToRemove += @(ResolveFilePaths -sourceFolder $templateFolder -originalSourceFolder $originalTemplateFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
$filesToRemove += @(ResolveFilePathsInDestinationFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
e2eTests/scenarios/CustomTemplate/runtest.ps1:364
- This assertion checks a custom text file, not a workflow file; the comment is misleading.
# Check that legacy workflow file is present in final repository
| $projects = @(GetProjectsFromRepository -baseFolder $baseFolder -projectsFromSettings $repoSettings.projects) | ||
|
|
||
| $filesToInclude, $filesToExclude = GetFilesToUpdate -settings $repoSettings -projects $projects -baseFolder $baseFolder -templateFolder $templateFolder -originalTemplateFolder $originalTemplateFolder | ||
| $filesToInclude, $filesToExclude, $filesToRemove = GetFilesToUpdate -settings $repoSettings -projects $projects -baseFolder $baseFolder -templateFolder $templateFolder -templateSettings $templateRepoSettings -originalTemplateFolder $originalTemplateFolder |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
Actions/CheckForUpdates/CheckForUpdates.HelperFunctions.ps1:1171
$settingsis already the effective result ofReadSettings, which includes the consumer's cached.github/AL-Go-TemplateRepoSettings.doNotEdit.json. Appending it after the live$templateSettingstherefore reintroduces stale template entries. If a custom template removes or changes an include/exclude/remove entry, the old entry remains active for this run (and a stalefilesToRemovecan still win), so the change still requires a second update. Pass consumer-only settings here, or letReadSettingsskip the cached template settings when live template settings are supplied.
if ($null -ne $templateSettings) {
$filesToIncludeUnresolved += $templateSettings.customALGoFiles.filesToInclude
}
$filesToIncludeUnresolved += $settings.customALGoFiles.filesToInclude
Actions/CheckForUpdates/CheckForUpdates.HelperFunctions.ps1:1202
- Template-resolved entries can construct
destinationFullPathfromdestinationFolder/destinationNamewithout checking that the result remains under$baseFolder. For example, a matching template file with a../destination can enter$filesToRemove, after whichCheckForUpdates.ps1resolves and removes that path. The source-prefix check inResolveFilePathsdoes not protect the destination. Canonicalize every removal destination and reject it unless it is within the repository root.
$filesToRemove = @(ResolveFilePaths -sourceFolder $templateFolder -originalSourceFolder $originalTemplateFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
if ($hasOriginalTemplate) {
$filesToRemove += @(ResolveFilePaths -sourceFolder $originalTemplateFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
}
$filesToRemove += @(ResolveFilePathsInDestinationFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
Actions/CheckForUpdates/CheckForUpdates.HelperFunctions.ps1:1210
- Validate every resolved removal destination against the canonical
$baseFolderboundary. AfilesToRemoveentry whosedestinationFolderordestinationNamecontains..can be resolved from a matching template file into a path outside the repository becauseResolveFilePathschecks only the source path. That path is subsequently accepted byCheckForUpdates.ps1and passed toRemove-Item, enabling deletion outside the cloned repository.
$filesToRemove = @(ResolveFilePaths -sourceFolder $templateFolder -originalSourceFolder $originalTemplateFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
if ($hasOriginalTemplate) {
$filesToRemove += @(ResolveFilePaths -sourceFolder $originalTemplateFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
}
$filesToRemove += @(ResolveFilePathsInDestinationFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
e2eTests/scenarios/CustomTemplate/runtest.ps1:454
- Use the possessive “repo's,” not “repos's.”
# Check that optional custom file is present (in repos's filesToInclude)
Actions/CheckForUpdates/CheckForUpdates.HelperFunctions.ps1:830
- Appending a separator does not make the containment check path-safe: line 885 still uses case-insensitive, wildcard-aware
-like. On Linux,sourceFolder = ../Repocan therefore pass the check for a base folder namedrepo, even though/tmp/Repois a distinct sibling; paths containing[or?are also interpreted as patterns. Canonicalize both paths and use a platform-appropriate ordinal prefix comparison (and a literal path for enumeration).
$sourceFolder = Join-Path $sourceFolder '' # Ensure source folder has a trailing slash for correct path resolution
| UpdateCustomTemplateRepoSettingsSnapshot -baseFolder $baseFolder -templateFolder $templateFolder | ||
| $repoSettings = ReadSettings -buildMode '' -project '' -workflowName '' -userName '' -branchName '' -trigger '' | ConvertTo-HashTable -recurse |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
Actions/CheckForUpdates/CheckForUpdates.ps1:124
- Refreshing the tracked snapshot in the comparison checkout makes it invisible to the update.
GetFilesToUpdatelater reads the same template file and compares it with this already-overwritten destination, so no$updateFilesentry is created;CloneIntoNewFolderthen clones the stale remote snapshot and never writes the refreshed content. The new E2E assertion atruntest.ps1:400will therefore fail, and other workflows continue reading stale template settings. Merge the fresh settings in memory, or explicitly carry the snapshot into the cloned update folder.
UpdateCustomTemplateRepoSettingsSnapshot -baseFolder $baseFolder -templateFolder $templateFolder
$repoSettings = ReadSettings -buildMode '' -project '' -workflowName '' -userName '' -branchName '' -trigger '' | ConvertTo-HashTable -recurse
Actions/CheckForUpdates/CheckForUpdates.HelperFunctions.ps1:1210
- These newly resolved removal entries can point outside
$baseFolder: a template-controlleddestinationFolderordestinationNamecontaining..is used to constructdestinationFullPathwithout a destination-boundary check. If the corresponding template source exists, CheckForUpdates later resolves and removes that path, allowing deletion outside the cloned repository (or causing the update to fail). Canonicalize each destination and reject any path outside the repository root before adding it tofilesToRemove.
$filesToRemove = @(ResolveFilePaths -sourceFolder $templateFolder -originalSourceFolder $originalTemplateFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
if ($hasOriginalTemplate) {
$filesToRemove += @(ResolveFilePaths -sourceFolder $originalTemplateFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
}
$filesToRemove += @(ResolveFilePathsInDestinationFolder -destinationFolder $baseFolder -files $filesToRemoveUnresolved -projects $projects)
Actions/CheckForUpdates/CheckForUpdates.HelperFunctions.ps1:1178
- At this point
ReadSettingshas already merged custom-template and consumer-repository arrays, so a template-only include triggers the “of repository” event and no template-specific telemetry is emitted. This does not provide the separate repository/template usage tracking described by the PR. Preserve the two sources until telemetry is recorded (or emit template telemetry before merging).
Trace-Information -Message "Usage: Custom AL-Go Files (Include) of repository"
}
if ($settings.customALGoFiles.filesToExclude.Count -gt 0) {
Trace-Information -Message "Usage: Custom AL-Go Files (Exclude) of repository"
| # Remove files that are in $filesToExclude and exist in the repository | ||
| $removeFiles = $filesToExclude | Where-Object { $_ -and (Test-Path -Path $_.destinationFullPath -PathType Leaf) } | ForEach-Object { | ||
| # Remove files that are in $filesToExclude or $filesToRemove and exist in the repository | ||
| $removeFiles = @($filesToExclude) + @($filesToRemove) | Where-Object { $_ -and (Test-Path -Path $_.destinationFullPath -PathType Leaf) } | ForEach-Object { |
| - originalSourceFullPath: The full path to the original source file in the original template repository (if any). | ||
| Always $null for filesToRemove entries. |
| # Check that missing workflow file is present (in default filesToExclude) | ||
| (Join-Path (Get-Location) $missingWorkflowFileRelativePath) | Should -Exist |
|
|
||
| - **`filesToInclude`** now also resolves files from the original AL-Go template. Files present in the official template are propagated even when they are absent from your custom template. When a file exists in both, the official template supplies the base content; for workflow files, customizations from the custom template are reapplied. | ||
| - **`filesToExclude`** now also resolves files from the original AL-Go template (same dual-resolution as `filesToInclude`). Files resolved by `filesToInclude` whose source matches a `filesToExclude` entry are not copied to consumer repos, and existing copies are removed. | ||
| - **`filesToRemove`** (new property): Unconditionally removes matching files from consumer repos. Files are searched in both the template and end repository. Takes precedence over `filesToInclude`. Entries use `sourceFolder` (relative to the template), `filter`, and optionally `destinationFolder`, `perProject`, and `destinationName`. |
There was a problem hiding this comment.
Let's skip filesToRemove for now.
❔What, Why & How
This pull request enhances the AL-Go template update system by introducing a new mechanism for unconditionally removing files from repositories, expanding file specification options, and improving the logic for determining which files to include, exclude, or remove during updates. The changes also improve documentation and schema validation, and add new helper functions to streamline file resolution.
Key changes:
1. File Removal Functionality
filesToRemovearray in settings, allowing unconditional removal of files from repositories during updates, regardless of whether they exist in the template. This is reflected in both the PowerShell logic and the settings schema. [1] [2] [3] [4]2. File Specification Enhancements
destinationNameproperty to file specifications, enabling files to be renamed when copied or removed, and updated the schema and logic accordingly. [1] [2]filesToIncludeandfilesToExcludein the schema, clarifying propagation and behavior. [1] [2]3. Update Logic Improvements
filesToRemoveare excluded from inclusion, and that exclusions are mapped more accurately. [1] [2]4. Helper Functions and Documentation
ResolveFilePathsInDestinationFolder, a helper for resolving file specs that refer to files already in the destination (e.g., for removal), with detailed documentation.5. Minor Path Handling Fixes
sourceFolderalways has a trailing slash in file path resolution to prevent path errors.Related to discussion: #2227
✅ Checklist