From f0be023d797f4d6dee9e9b4114ac8df5e87ae38d Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Thu, 8 Oct 2026 21:04:58 +0200 Subject: [PATCH 1/2] Let update.check in the settings turn the Validate update check off An organization that wanted the template update check off had to edit the checkForUpdates input in Validate.yaml, a system file the next update overwrites (#67). The settings survive updates, so the switch belongs there. The settings schema gains update.check (boolean, default true; the schema stays closed). The Validate action input checkForUpdates now defaults to '' and Validate.ps1 takes it as a tri-state string: 'true' or 'false' (case-insensitive, $true and $false from in-process callers too) wins, '' follows update.check, read leniently so a missing or unreadable settings file or a non-boolean value leaves the check on. When the setting turns it off the log says "Update check off (update.check is false)" and UpdateCheck stays $null. The template settings do not ship the key (absent means true); the engine CI already passes checkForUpdates 'false' explicitly. Tests: the action input default, Invoke-Entry defaulting to 'false' so no test downloads a template, four precedence cases on update-org copies and two schema fixtures. Docs: update-mechanics section 7, ARCHITECTURE 5.5 and section 8, the template README. Co-Authored-By: Claude Fable 5.1 --- actions/Validate/Validate.ps1 | 28 ++++++++-- actions/Validate/action.yaml | 6 +-- docs/ARCHITECTURE.md | 6 +-- docs/reference/update-mechanics.md | 4 +- schemas/rulebook-settings.schema.json | 3 +- template/README.md | 2 +- tests/Validate.Action.Tests.ps1 | 53 ++++++++++++++++++- .../update-check-not-boolean.json | 9 ++++ .../rulebook-settings/update-check-false.json | 9 ++++ 9 files changed, 104 insertions(+), 16 deletions(-) create mode 100644 tests/fixtures/schemas/invalid/rulebook-settings/update-check-not-boolean.json create mode 100644 tests/fixtures/schemas/valid/rulebook-settings/update-check-false.json diff --git a/actions/Validate/Validate.ps1 b/actions/Validate/Validate.ps1 index 39d3690..5b8c9c6 100644 --- a/actions/Validate/Validate.ps1 +++ b/actions/Validate/Validate.ps1 @@ -8,9 +8,10 @@ appends a Markdown summary to -SummaryPath, writes the errors and warnings outpu { ExitCode, Findings, Diff, Summary, Annotations, DiffRef, UpdateCheck }. ExitCode is 1 when there are errors, or warnings with -FailOnWarning. The script never calls exit, so tests run it in-process; action.yaml exits with ExitCode. --CheckForUpdates runs the template update check in check mode (Rulebook.Update): the template of the settings at the -head of its branch, downloaded with GITHUB_TOKEN (never the write token), or -TemplatePath and -InstalledTemplatePath -as local folders. One notice (no updates, templateSha not recorded) or warning (updates available, check skipped) and +-CheckForUpdates 'true' or 'false' (case-insensitive; $true and $false work too) runs or skips the template update +check; '' (the default) follows update.check of the settings, true when absent. The check runs in check mode +(Rulebook.Update): the template of the settings at the head of its branch, downloaded with GITHUB_TOKEN (never the +write token), or -TemplatePath and -InstalledTemplatePath as local folders. One notice (no updates, templateSha not recorded) or warning (updates available, check skipped) and the section '## Template update check' in the summary. Neither counts towards warnings= or -FailOnWarning, and the check never fails the step. UpdateCheck is { Status (none, sha-only, available, skipped), Reason, Plan }. The check works in -UpdateWorkPath when given (left in place afterwards), else in a temporary folder it removes. @@ -24,7 +25,7 @@ event, a ref that does not resolve, or a diff that fails is a note in the summar param( [string]$RepositoryRoot = '.', [switch]$FailOnWarning, - [switch]$CheckForUpdates, + [AllowEmptyString()][string]$CheckForUpdates = '', [AllowEmptyString()][string]$DiffRef, [string]$SummaryPath = $env:GITHUB_STEP_SUMMARY, [string]$JsonPath, @@ -57,6 +58,23 @@ $workspace = if ([string]::IsNullOrEmpty($WorkspaceRoot)) { $root } else { (Reso $separator = [System.IO.Path]::DirectorySeparatorChar $relativeRoot = [System.IO.Path]::GetRelativePath($workspace, $root).Replace($separator, '/') +# The update check: an explicit -CheckForUpdates wins; '' follows update.check of the settings, read leniently (a +# missing or unreadable settings file, or a value that is not a boolean, leaves the check on; the checks report them). +$runUpdateCheck = $true +if (-not [string]::IsNullOrWhiteSpace($CheckForUpdates)) { + $runUpdateCheck = $CheckForUpdates.Trim() -ieq 'true' +} else { + try { + $settingsForCheck = Get-Content -LiteralPath (Join-Path $root '.github' 'Rulebook-Settings.json') -Raw -ErrorAction Stop | ConvertFrom-Json -AsHashtable -ErrorAction Stop + if ($settingsForCheck -is [System.Collections.IDictionary] -and $settingsForCheck['update'] -is [System.Collections.IDictionary] -and $settingsForCheck['update']['check'] -is [bool]) { + $runUpdateCheck = $settingsForCheck['update']['check'] + } + } catch { + Write-Verbose "Settings not readable for update.check: $($_.Exception.Message)" + } + if (-not $runUpdateCheck) { Write-Host 'Update check off (update.check is false)' } +} + # 1. Checks $testParameters = @{ RepositoryRoot = $root } if ($JsonPath) { $testParameters.Json = $JsonPath } @@ -176,7 +194,7 @@ Write-Text -Path $SummaryPath -Text $summaryText # 5. Update check (WP07): check mode only, never counted, never failing. $updateCheck = $null -if ($CheckForUpdates) { +if ($runUpdateCheck) { $updateCheck = [pscustomobject]@{ Status = 'skipped'; Reason = $null; Plan = $null } $command = 'warning' $message = $null diff --git a/actions/Validate/action.yaml b/actions/Validate/action.yaml index f2ddd10..eba97d7 100644 --- a/actions/Validate/action.yaml +++ b/actions/Validate/action.yaml @@ -12,9 +12,9 @@ inputs: required: false default: 'false' checkForUpdates: - description: "'true' runs the template update check in check mode: one notice or warning, never counted towards warnings or failOnWarning." + description: "'true' or 'false' runs or skips the template update check; empty follows update.check of the settings (default true)." required: false - default: 'true' + default: '' outputs: errors: @@ -41,7 +41,7 @@ runs: $parameters = @{ RepositoryRoot = $env:INPUT_REPOSITORYROOT FailOnWarning = $env:INPUT_FAILONWARNING -eq 'true' - CheckForUpdates = $env:INPUT_CHECKFORUPDATES -eq 'true' + CheckForUpdates = $env:INPUT_CHECKFORUPDATES } # The result object is the last pipeline output; anything a module emits before it is ignored. $result = @(& (Join-Path $env:GITHUB_ACTION_PATH 'Validate.ps1') @parameters)[-1] diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 9a55ce1..7bfbde9 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -242,7 +242,7 @@ A rule is `id` (`^[A-Z]{2,3}[0-9]{4}i?$`), `action` (`Error`, `Warning`, `Info`, **Validate action** -`actions/Validate/action.yaml` is a composite action. Inputs: `repositoryRoot` (default `.`), `failOnWarning` (default `'false'`), `checkForUpdates` (default `'true'`, the template update check of section 7.3 in check mode). Outputs: `errors`, `warnings`. One `pwsh` step receives the inputs and the read-only `GITHUB_TOKEN` through `env:` and runs `Validate.ps1` from `GITHUB_ACTION_PATH`, which imports the modules from `../../modules` of the same engine checkout, and exits with its `ExitCode`. Like the entry scripts of the other actions, `Validate.ps1` writes its annotations, summary and outputs through `modules/Rulebook.Action` (#58), a leaf module without engine imports. +`actions/Validate/action.yaml` is a composite action. Inputs: `repositoryRoot` (default `.`), `failOnWarning` (default `'false'`), `checkForUpdates` (default `''`: the template update check of section 7.3 in check mode runs unless `update.check` of the settings is `false`; `'true'` or `'false'` overrides the setting). Outputs: `errors`, `warnings`. One `pwsh` step receives the inputs and the read-only `GITHUB_TOKEN` through `env:` and runs `Validate.ps1` from `GITHUB_ACTION_PATH`, which imports the modules from `../../modules` of the same engine checkout, and exits with its `ExitCode`. Like the entry scripts of the other actions, `Validate.ps1` writes its annotations, summary and outputs through `modules/Rulebook.Action` (#58), a leaf module without engine imports. `Validate.ps1` (`-RepositoryRoot`, `-FailOnWarning`, `-CheckForUpdates`, `-DiffRef`, `-SummaryPath`, `-JsonPath`, `-WorkspaceRoot`, and the test seams `-TemplatePath`, `-InstalledTemplatePath`, `-ApiUrl`, `-UpdateWorkPath`) never calls `exit` and returns `{ ExitCode, Findings, Diff, Summary, Annotations, DiffRef, UpdateCheck }`: @@ -250,7 +250,7 @@ A rule is `id` (`^[A-Z]{2,3}[0-9]{4}i?$`), `action` (`Error`, `Warning`, `Info`, 2. The effective diff against `-DiffRef`. The default is the pull request's base branch on a pull request (fetched when absent); on a push, the commit before the push from the event payload, else the last commit (`HEAD~1`); other events get no diff. `-DiffRef ''` disables it. A ref that does not resolve, or a failing comparison, is a note in the summary, never a failure. 3. The job summary: `## Rulebook validation` with the counts and a `| Rule | Severity | File | Id | Message |` table (or "No findings."), then `## Effective diff against ` with one `| Id | Before | After | Decided by |` table per changed endpoint (or "No effective change."). 4. `errors=` and `warnings=` to `GITHUB_OUTPUT`. `ExitCode` is 1 on any error, or on any warning with `failOnWarning`. -5. With `-CheckForUpdates`: the update plan against the template of the settings at the head of its branch (downloaded with `GITHUB_TOKEN`, never the write token), one notice (no updates, templateSha not recorded) or warning (updates available, or `update check skipped: `) titled `Update check`, and the section `## Template update check` in the summary. Neither annotation counts towards `warnings=` or `failOnWarning`, and the check never fails the step. `UpdateCheck` is `{ Status (none, sha-only, available, skipped), Reason, Plan }`. Details: [reference/update-mechanics.md](reference/update-mechanics.md) section 7. +5. With the update check on (`-CheckForUpdates 'true'`, or `''` and `update.check` not `false`; when the setting turns it off the log says `Update check off (update.check is false)`): the update plan against the template of the settings at the head of its branch (downloaded with `GITHUB_TOKEN`, never the write token), one notice (no updates, templateSha not recorded) or warning (updates available, or `update check skipped: `) titled `Update check`, and the section `## Template update check` in the summary. Neither annotation counts towards `warnings=` or `failOnWarning`, and the check never fails the step. `UpdateCheck` is `{ Status (none, sha-only, available, skipped), Reason, Plan }`. Details: [reference/update-mechanics.md](reference/update-mechanics.md) section 7. ### 5.6 Template generators @@ -518,7 +518,7 @@ A change set is `{ version, note?, changes[] }` with `set` (write an override en `site` (D31, D35, D36): `enabled` builds and publishes the dashboard (default true; ignored with a warning for `dist-repo` and `gist`); `includeJustifications` publishes the organization's override and quarantine justification text (default false); `updateMode` is `skip` (keep locally changed site files on update) or `overwrite`. -`update` (WP07) and `scan` (WP08): `schedule` is a five-field cron string that the update writes into `UpdateRulebookSystemFiles.yaml` (`update.schedule`, shipped `null`: no schedule) or `ScanDiagnostics.yaml` (`scan.schedule`, shipped `"17 4 * * *"`: daily at 04:17 UTC) as its `schedule:` trigger, or `null` for none. An absent `update.schedule` removes the update schedule; an absent `scan` key or `scan.schedule` keeps the scan schedule the template ships (an organization from before WP08 has no `scan` key), and only an explicit `null` removes it. `unusedRulebookFiles` lists shipped files the update must not re-add and removes when present, by their repository-relative path with `/` (`stages/vnext.json`); the schema rejects a bare file name, a backslash and a leading slash. `templateSha` is written by every update; `templateUrl` is the template the next update pulls from. +`update` (WP07) and `scan` (WP08): `schedule` is a five-field cron string that the update writes into `UpdateRulebookSystemFiles.yaml` (`update.schedule`, shipped `null`: no schedule) or `ScanDiagnostics.yaml` (`scan.schedule`, shipped `"17 4 * * *"`: daily at 04:17 UTC) as its `schedule:` trigger, or `null` for none. An absent `update.schedule` removes the update schedule; an absent `scan` key or `scan.schedule` keeps the scan schedule the template ships (an organization from before WP08 has no `scan` key), and only an explicit `null` removes it. `update.check` (boolean, default `true`, not shipped) set to `false` turns the template update check of the Validate action off; an explicit `checkForUpdates` input still wins (#67). `unusedRulebookFiles` lists shipped files the update must not re-add and removes when present, by their repository-relative path with `/` (`stages/vnext.json`); the schema rejects a bare file name, a backslash and a leading slash. `templateSha` is written by every update; `templateUrl` is the template the next update pulls from. The schema is closed: an unknown key at the top level or in `publish`, `quarantine`, `commitOptions`, `site`, `update` or `scan` is an error, and a work package that needs a new key adds it to the schema in its own pull request. Required are `templateUrl`, `baseUrl` (empty, or `https://` with a DNS host name and an optional numeric port, no user info, and without a query, a fragment, `.` or `..` segments, quotes, backslashes, control characters or a trailing slash), `publish`, `quarantine` with both keys, `levels` and `stages`. `publish.target` requires its own fields: `dist-repo` needs `repository` (`owner/name`) and `branch`, `azure-blob` needs `storageAccount` and `container`, `gist` needs `gistId`; fields of another target are allowed and ignored. `templateSha` is empty or a 40-character commit sha. A level or stage entry has a `name` matching `^[A-Za-z0-9-]+$` and an optional `description`; a level may have `basedOn` with the same pattern. Beyond the schema, C5 checks that slugs are unique per array after lowercasing, that every `basedOn` resolves to an existing `base/.ruleset.json` without a cycle, and that quarantine values are stage slugs from the settings. diff --git a/docs/reference/update-mechanics.md b/docs/reference/update-mechanics.md index 75bc83a..abaa13b 100644 --- a/docs/reference/update-mechanics.md +++ b/docs/reference/update-mechanics.md @@ -110,7 +110,7 @@ Line endings become LF. Keys the engine does not know survive; the schema reject ## 7. Check mode -The Validate action runs the check when `checkForUpdates` is `'true'` (the default); CheckForUpdates runs it with `update` other than `'Y'`. Validate downloads the template of the settings at the head of its branch, and both read it with `GITHUB_TOKEN` only (CheckForUpdates passes no write token in check mode); a template that token cannot read (a private template, or a pull request from a fork with a restricted token) gives "update check skipped". +The Validate action runs the check when `checkForUpdates` is `'true'`, or when it is empty (the default) and `update.check` of the settings is not `false`; CheckForUpdates runs it with `update` other than `'Y'`. Validate downloads the template of the settings at the head of its branch, and both read it with `GITHUB_TOKEN` only (CheckForUpdates passes no write token in check mode); a template that token cannot read (a private template, or a pull request from a fork with a restricted token) gives "update check skipped". | Outcome | Annotation | |---|---| @@ -122,6 +122,8 @@ The Validate action runs the check when `checkForUpdates` is `'true'` (the defau Neither annotation counts towards `warnings=` or `failOnWarning`, and the check never fails the step. The engine CI passes `checkForUpdates: 'false'` on its fixture steps. +**Turning it off (#67).** `update.check: false` in `.github/Rulebook-Settings.json` turns the check off for every Validate run; the log says `Update check off (update.check is false)` and the summary has no update section. The key lives in the settings, so it survives updates, where an edited `checkForUpdates` input in `Validate.yaml` (a system file) would be overwritten. The template does not ship the key; absent means `true`. Precedence: an explicit `checkForUpdates` input (`'true'` or `'false'`) wins over the setting, an empty input follows it. Validate reads the setting leniently: a missing or unreadable settings file, or a value that is not a boolean, leaves the check on (the checks report the settings themselves). + ## 8. Update mode 1. No token: `failure=token`, error `The secret is needed to update system files. Read https://github.com/ALCops/rulebook/blob/main/docs/ghtokenworkflow.md`, before any request. diff --git a/schemas/rulebook-settings.schema.json b/schemas/rulebook-settings.schema.json index ba98dd7..d8de0f0 100644 --- a/schemas/rulebook-settings.schema.json +++ b/schemas/rulebook-settings.schema.json @@ -150,7 +150,8 @@ "description": "The update workflow (WP07): schedule is a five-field cron expression the update writes into UpdateRulebookSystemFiles.yaml as its schedule trigger, or null for none.", "type": "object", "properties": { - "schedule": { "type": ["string", "null"], "pattern": "^\\S+( \\S+){4}(?![\\s\\S])" } + "schedule": { "type": ["string", "null"], "pattern": "^\\S+( \\S+){4}(?![\\s\\S])" }, + "check": { "description": "false turns the template update check of the Validate action off; an explicit checkForUpdates input still wins.", "type": "boolean", "default": true } }, "additionalProperties": false }, diff --git a/template/README.md b/template/README.md index fbd0149..5003cb1 100644 --- a/template/README.md +++ b/template/README.md @@ -24,7 +24,7 @@ System files are replaced when you update from the template; settings are kept; ## Updating -The workflow **Update Rulebook System Files** (Actions > Update Rulebook System Files > Run workflow) pulls the newest version of the template into a pull request: the system files, your settings with the new `templateSha`, and `rulesets/` and `skeletons/` regenerated under your overrides and quarantine, with the effective change of every endpoint in the pull request. Your own files are never touched. It needs a secret `GHTOKENWORKFLOW` (a GitHub App or a personal access token, the same secret AL-Go uses), because the workflow token cannot change workflow files. While `checkForUpdates` is on (the default), every Validate run also tells you when an update is available; when the template cannot be read with the workflow token (a private template, or a pull request from a fork) it says "update check skipped" instead. How it works: [docs/updating.md](https://github.com/ALCops/rulebook/blob/main/docs/updating.md); the secret: [docs/ghtokenworkflow.md](https://github.com/ALCops/rulebook/blob/main/docs/ghtokenworkflow.md). +The workflow **Update Rulebook System Files** (Actions > Update Rulebook System Files > Run workflow) pulls the newest version of the template into a pull request: the system files, your settings with the new `templateSha`, and `rulesets/` and `skeletons/` regenerated under your overrides and quarantine, with the effective change of every endpoint in the pull request. Your own files are never touched. It needs a secret `GHTOKENWORKFLOW` (a GitHub App or a personal access token, the same secret AL-Go uses), because the workflow token cannot change workflow files. Every Validate run also tells you when an update is available (set `"update": { "check": false }` in `.github/Rulebook-Settings.json` to turn that off); when the template cannot be read with the workflow token (a private template, or a pull request from a fork) it says "update check skipped" instead. How it works: [docs/updating.md](https://github.com/ALCops/rulebook/blob/main/docs/updating.md); the secret: [docs/ghtokenworkflow.md](https://github.com/ALCops/rulebook/blob/main/docs/ghtokenworkflow.md). ## First steps diff --git a/tests/Validate.Action.Tests.ps1 b/tests/Validate.Action.Tests.ps1 index cf542f5..62d08f3 100644 --- a/tests/Validate.Action.Tests.ps1 +++ b/tests/Validate.Action.Tests.ps1 @@ -22,6 +22,8 @@ BeforeAll { function Invoke-Entry { # Runs Validate.ps1 in-process; returns the result object and the console lines (information stream). param([hashtable]$Parameters) + # The update check follows update.check by default; a test that does not ask for it never downloads a template. + if (-not $Parameters.ContainsKey('CheckForUpdates')) { $Parameters.CheckForUpdates = 'false' } if (-not $Parameters.ContainsKey('SummaryPath')) { $Parameters.SummaryPath = Join-Path $TestDrive ('summary-{0}.md' -f [guid]::NewGuid().ToString('n')) } # Derive = $true leaves -DiffRef unbound, so Validate.ps1 derives it from the event. if ($Parameters.ContainsKey('Derive')) { $Parameters.Remove('Derive') } elseif (-not $Parameters.ContainsKey('DiffRef')) { $Parameters.DiffRef = '' } @@ -55,7 +57,7 @@ Describe 'actions/Validate/action.yaml' { It 'declares input with default ' -ForEach @( @{ Name = 'repositoryRoot'; Default = "'.'" } @{ Name = 'failOnWarning'; Default = "'false'" } - @{ Name = 'checkForUpdates'; Default = "'true'" } + @{ Name = 'checkForUpdates'; Default = "''" } ) { $yaml | Should-MatchString ("(?ms)^ {0}:\n.*?^ default: {1}$" -f $Name, [regex]::Escape($Default)) } @@ -238,13 +240,60 @@ Describe 'Validate.ps1' { [System.Text.Encoding]::UTF8.GetByteCount($run.Summary) | Should-BeLessThanOrEqual $limit } - It 'runs no update check without -CheckForUpdates' { + It 'runs no update check when the input is false' { $run = Invoke-Entry @{ RepositoryRoot = $org } $run.Result.UpdateCheck | Should-BeNull $run.Summary | Should-NotMatchString 'Template update check' } } + Context 'update.check of the settings (#67)' { + BeforeAll { + $script:v1 = Join-Path $PSScriptRoot 'fixtures' 'templates' 'v1' + $script:v2 = Join-Path $PSScriptRoot 'fixtures' 'templates' 'v2' + + function Copy-UpdateOrg { + # A copy of update-org; -Check writes update.check into its settings, otherwise the key stays absent. + param([AllowNull()][object]$Check) + $root = New-FixtureRepo -Name 'update-org' -Destination (Join-Path $TestDrive ([guid]::NewGuid().ToString('n').Substring(0, 12))) + if ($PSBoundParameters.ContainsKey('Check')) { + $settingsFile = Join-Path $root '.github' 'Rulebook-Settings.json' + $text = Get-Content -LiteralPath $settingsFile -Raw + $updated = $text.Replace('"update": { "schedule": "0 6 * * 1" }', ('"update": {{ "schedule": "0 6 * * 1", "check": {0} }}' -f $Check.ToString().ToLowerInvariant())) + $updated | Should-NotBe $text + Write-FixtureText -Path $settingsFile -Text $updated + } + return $root + } + } + + It 'runs the check when the input is empty and update.check is absent' { + $run = Invoke-Entry @{ RepositoryRoot = (Copy-UpdateOrg); CheckForUpdates = ''; TemplatePath = $v2; InstalledTemplatePath = $v1 } + $run.Result.UpdateCheck.Status | Should-Be 'available' + $run.Lines | Should-NotContainCollection @('Update check off (update.check is false)') + } + + It 'turns the check off with update.check false and says so in the log' { + $run = Invoke-Entry @{ RepositoryRoot = (Copy-UpdateOrg -Check $false); CheckForUpdates = ''; TemplatePath = $v2; InstalledTemplatePath = $v1 } + $run.Result.ExitCode | Should-Be 0 + $run.Result.UpdateCheck | Should-BeNull + $run.Lines | Should-ContainCollection @('Update check off (update.check is false)') + $run.Summary | Should-NotMatchString 'Template update check' + } + + It 'runs the check for an explicit true input although update.check is false' { + $run = Invoke-Entry @{ RepositoryRoot = (Copy-UpdateOrg -Check $false); CheckForUpdates = 'True'; TemplatePath = $v2; InstalledTemplatePath = $v1 } + $run.Result.UpdateCheck.Status | Should-Be 'available' + $run.Lines | Should-NotContainCollection @('Update check off (update.check is false)') + } + + It 'skips the check for an explicit false input although update.check is true' { + $run = Invoke-Entry @{ RepositoryRoot = (Copy-UpdateOrg -Check $true); CheckForUpdates = 'false'; TemplatePath = $v2; InstalledTemplatePath = $v1 } + $run.Result.UpdateCheck | Should-BeNull + $run.Summary | Should-NotMatchString 'Template update check' + } + } + It 'shows the effective diff against -DiffRef HEAD~1' -Skip:$gitMissing { $root = New-FixtureRepo -Name 'valid-minimal' -Destination (Join-Path $TestDrive 'diff') $null = New-FixtureGitRepo -Root $root -Message 'before' diff --git a/tests/fixtures/schemas/invalid/rulebook-settings/update-check-not-boolean.json b/tests/fixtures/schemas/invalid/rulebook-settings/update-check-not-boolean.json new file mode 100644 index 0000000..ea36c62 --- /dev/null +++ b/tests/fixtures/schemas/invalid/rulebook-settings/update-check-not-boolean.json @@ -0,0 +1,9 @@ +{ + "templateUrl": "https://github.com/ALCops/rulebook@main", + "baseUrl": "https://contoso.github.io/rulebook", + "publish": { "target": "pages" }, + "quarantine": { "stages": null, "prereleaseStages": null }, + "levels": [ { "name": "Essential" } ], + "stages": [ { "name": "default" } ], + "update": { "check": "false" } +} diff --git a/tests/fixtures/schemas/valid/rulebook-settings/update-check-false.json b/tests/fixtures/schemas/valid/rulebook-settings/update-check-false.json new file mode 100644 index 0000000..a327d01 --- /dev/null +++ b/tests/fixtures/schemas/valid/rulebook-settings/update-check-false.json @@ -0,0 +1,9 @@ +{ + "templateUrl": "https://github.com/ALCops/rulebook@main", + "baseUrl": "https://contoso.github.io/rulebook", + "publish": { "target": "pages" }, + "quarantine": { "stages": null, "prereleaseStages": null }, + "levels": [ { "name": "Essential" } ], + "stages": [ { "name": "default" } ], + "update": { "check": false } +} From dfd1fb3dfcee94fd825b35a27cfc3411b07c2814 Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Thu, 8 Oct 2026 21:15:14 +0200 Subject: [PATCH 2/2] Document the omitted -CheckForUpdates and log an unrecognised value Review round 1 of #67. The Validate.ps1 help and update-mechanics section 7 say that omitting -CheckForUpdates follows update.check (pass 'false' to skip). An explicit value other than 'true' or 'false' still turns the check off, now with the log line "checkForUpdates '' is not 'true' or 'false'; the update check is off" (no annotation, no exit-code effect). Tests cover the omitted parameter with and without update.check and the unrecognised value. Co-Authored-By: Claude Fable 5.1 --- actions/Validate/Validate.ps1 | 13 ++++++++----- docs/reference/update-mechanics.md | 2 +- tests/Validate.Action.Tests.ps1 | 21 ++++++++++++++++++++- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/actions/Validate/Validate.ps1 b/actions/Validate/Validate.ps1 index 5b8c9c6..cd14b76 100644 --- a/actions/Validate/Validate.ps1 +++ b/actions/Validate/Validate.ps1 @@ -9,11 +9,13 @@ appends a Markdown summary to -SummaryPath, writes the errors and warnings outpu warnings with -FailOnWarning. The script never calls exit, so tests run it in-process; action.yaml exits with ExitCode. -CheckForUpdates 'true' or 'false' (case-insensitive; $true and $false work too) runs or skips the template update -check; '' (the default) follows update.check of the settings, true when absent. The check runs in check mode -(Rulebook.Update): the template of the settings at the head of its branch, downloaded with GITHUB_TOKEN (never the -write token), or -TemplatePath and -InstalledTemplatePath as local folders. One notice (no updates, templateSha not recorded) or warning (updates available, check skipped) and -the section '## Template update check' in the summary. Neither counts towards warnings= or -FailOnWarning, and the -check never fails the step. UpdateCheck is { Status (none, sha-only, available, skipped), Reason, Plan }. The check works +check; '' (the default) follows update.check of the settings, true when absent. Omitting -CheckForUpdates follows +update.check of the settings; pass 'false' to skip the check. Any other value turns the check off and says so in the +log. The check runs in check mode (Rulebook.Update): the template of the settings at the head of its branch, +downloaded with GITHUB_TOKEN (never the write token), or -TemplatePath and -InstalledTemplatePath as local folders. +One notice (no updates, templateSha not recorded) or warning (updates available, check skipped) and the section +'## Template update check' in the summary. Neither counts towards warnings= or -FailOnWarning, and the check never +fails the step. UpdateCheck is { Status (none, sha-only, available, skipped), Reason, Plan }. The check works in -UpdateWorkPath when given (left in place afterwards), else in a temporary folder it removes. The effective diff compares against -DiffRef. Without -DiffRef it is origin/ on a pull_request or @@ -63,6 +65,7 @@ $relativeRoot = [System.IO.Path]::GetRelativePath($workspace, $root).Replace($se $runUpdateCheck = $true if (-not [string]::IsNullOrWhiteSpace($CheckForUpdates)) { $runUpdateCheck = $CheckForUpdates.Trim() -ieq 'true' + if ($CheckForUpdates.Trim() -inotin 'true', 'false') { Write-Host "checkForUpdates '$CheckForUpdates' is not 'true' or 'false'; the update check is off" } } else { try { $settingsForCheck = Get-Content -LiteralPath (Join-Path $root '.github' 'Rulebook-Settings.json') -Raw -ErrorAction Stop | ConvertFrom-Json -AsHashtable -ErrorAction Stop diff --git a/docs/reference/update-mechanics.md b/docs/reference/update-mechanics.md index abaa13b..a600057 100644 --- a/docs/reference/update-mechanics.md +++ b/docs/reference/update-mechanics.md @@ -122,7 +122,7 @@ The Validate action runs the check when `checkForUpdates` is `'true'`, or when i Neither annotation counts towards `warnings=` or `failOnWarning`, and the check never fails the step. The engine CI passes `checkForUpdates: 'false'` on its fixture steps. -**Turning it off (#67).** `update.check: false` in `.github/Rulebook-Settings.json` turns the check off for every Validate run; the log says `Update check off (update.check is false)` and the summary has no update section. The key lives in the settings, so it survives updates, where an edited `checkForUpdates` input in `Validate.yaml` (a system file) would be overwritten. The template does not ship the key; absent means `true`. Precedence: an explicit `checkForUpdates` input (`'true'` or `'false'`) wins over the setting, an empty input follows it. Validate reads the setting leniently: a missing or unreadable settings file, or a value that is not a boolean, leaves the check on (the checks report the settings themselves). +**Turning it off (#67).** `update.check: false` in `.github/Rulebook-Settings.json` turns the check off for every Validate run; the log says `Update check off (update.check is false)` and the summary has no update section. The key lives in the settings, so it survives updates, where an edited `checkForUpdates` input in `Validate.yaml` (a system file) would be overwritten. The template does not ship the key; absent means `true`. Precedence: an explicit `checkForUpdates` input (`'true'` or `'false'`) wins over the setting, an empty input follows it. Validate reads the setting leniently: a missing or unreadable settings file, or a value that is not a boolean, leaves the check on (the checks report the settings themselves). A caller of `Validate.ps1` that omits `-CheckForUpdates` follows the setting too; pass `'false'` to skip the check. Any other non-empty input turns the check off with the log line `checkForUpdates '' is not 'true' or 'false'; the update check is off`. ## 8. Update mode diff --git a/tests/Validate.Action.Tests.ps1 b/tests/Validate.Action.Tests.ps1 index 62d08f3..3de563f 100644 --- a/tests/Validate.Action.Tests.ps1 +++ b/tests/Validate.Action.Tests.ps1 @@ -23,7 +23,8 @@ BeforeAll { # Runs Validate.ps1 in-process; returns the result object and the console lines (information stream). param([hashtable]$Parameters) # The update check follows update.check by default; a test that does not ask for it never downloads a template. - if (-not $Parameters.ContainsKey('CheckForUpdates')) { $Parameters.CheckForUpdates = 'false' } + # OmitCheck = $true leaves -CheckForUpdates unbound, so Validate.ps1 follows update.check. + if ($Parameters.ContainsKey('OmitCheck')) { $Parameters.Remove('OmitCheck') } elseif (-not $Parameters.ContainsKey('CheckForUpdates')) { $Parameters.CheckForUpdates = 'false' } if (-not $Parameters.ContainsKey('SummaryPath')) { $Parameters.SummaryPath = Join-Path $TestDrive ('summary-{0}.md' -f [guid]::NewGuid().ToString('n')) } # Derive = $true leaves -DiffRef unbound, so Validate.ps1 derives it from the event. if ($Parameters.ContainsKey('Derive')) { $Parameters.Remove('Derive') } elseif (-not $Parameters.ContainsKey('DiffRef')) { $Parameters.DiffRef = '' } @@ -287,6 +288,24 @@ Describe 'Validate.ps1' { $run.Lines | Should-NotContainCollection @('Update check off (update.check is false)') } + It 'follows update.check false when the caller omits -CheckForUpdates' { + $run = Invoke-Entry @{ RepositoryRoot = (Copy-UpdateOrg -Check $false); OmitCheck = $true; TemplatePath = $v2; InstalledTemplatePath = $v1 } + $run.Result.UpdateCheck | Should-BeNull + $run.Lines | Should-ContainCollection @('Update check off (update.check is false)') + } + + It 'runs the check when the caller omits -CheckForUpdates and update.check is absent' { + $run = Invoke-Entry @{ RepositoryRoot = (Copy-UpdateOrg); OmitCheck = $true; TemplatePath = $v2; InstalledTemplatePath = $v1 } + $run.Result.UpdateCheck.Status | Should-Be 'available' + } + + It 'turns the check off and says so for an input other than true or false' { + $run = Invoke-Entry @{ RepositoryRoot = (Copy-UpdateOrg -Check $true); CheckForUpdates = 'yes'; TemplatePath = $v2; InstalledTemplatePath = $v1 } + $run.Result.ExitCode | Should-Be 0 + $run.Result.UpdateCheck | Should-BeNull + $run.Lines | Should-ContainCollection @("checkForUpdates 'yes' is not 'true' or 'false'; the update check is off") + } + It 'skips the check for an explicit false input although update.check is true' { $run = Invoke-Entry @{ RepositoryRoot = (Copy-UpdateOrg -Check $true); CheckForUpdates = 'false'; TemplatePath = $v2; InstalledTemplatePath = $v1 } $run.Result.UpdateCheck | Should-BeNull