Skip to content

Remove PR-commenting side effect from Get-PSModuleSettings action #323

@MariusStorhaug

Description

The Get-PSModuleSettings action is consumed by Get-Settings.yml as a read-only configuration loader. However, it posts comments to pull requests as a side effect when build/test stages are skipped due to no important file changes.

Request

Current experience

Get-PSModuleSettings/src/main.ps1 (around lines 295–332) contains logic that posts a PR comment explaining why stages were skipped:

if ($isOpenOrUpdatedPR) {
    Write-Host 'Adding comment to PR about skipped stages...'
    $apiParams = @{
        Method      = 'POST'
        ApiEndpoint = "/repos/$owner/$repo/issues/$prNumber/comments"
        Body        = @{ body = $commentBody } | ConvertTo-Json
    }
    $null = Invoke-GitHubAPI @apiParams
}

This is why Get-Settings.yml requires pull-requests: write permission — a surprising requirement for a settings-reading workflow. A maintainer inspecting permissions would not expect a "get settings" step to write to PRs.

Desired experience

Get-PSModuleSettings is purely read-only: it reads the settings file, validates it, resolves configuration, and outputs a JSON object. Any PR communication (comments, labels, notices) is handled by the calling workflow or a dedicated notification step.

Acceptance criteria

  • Get-PSModuleSettings action produces only read-only outputs (settings JSON)
  • No API calls that mutate repository state (comments, labels) originate from the settings action
  • Get-Settings.yml no longer requires pull-requests: write permission
  • The "skipped stages" notification is preserved — either as a separate workflow step or via GitHub step summary
  • No change in user-visible behavior (maintainers still learn why stages were skipped)

Note

This violates the Single Responsibility Principle (SRP): a settings reader should not have the side effect of writing PR comments. It also violates the principle of least privilege by requiring write permissions for a read operation.


Technical decisions

Approach: Remove the PR commenting logic from Get-PSModuleSettings/src/main.ps1. Instead, add the skipped-stages information as a structured field in the Settings output (e.g., Settings.SkippedStagesMessage). The calling workflow (workflow.yml or Get-Settings.yml) can then decide how to surface this information — via a separate step that posts a comment, via GitHub step summary, or via ::notice::.

Alternative considered: Making the comment opt-in via an input flag (e.g., PostSkipComment: true). Rejected because it preserves the SRP violation — the action still contains PR-commenting code, even if gated.

Permission impact: Get-Settings.yml can drop from pull-requests: write to pull-requests: read (or remove the permission entirely if labels are also handled elsewhere).


Implementation plan

Get-PSModuleSettings action changes

  • Add a SkippedStagesMessage field to the Settings output object containing the notification text (when stages are skipped)
  • Remove the Invoke-GitHubAPI call that posts PR comments from src/main.ps1
  • Remove any PR label-writing logic if present

Get-Settings.yml changes

  • Reduce pull-requests permission from write to read
  • Optionally add a step after Get-Settings that posts the skipped-stages comment using the output field

workflow.yml changes

  • If the notification is moved to workflow.yml, add a lightweight job or step that posts the comment using the Settings output

Testing

  • Verify settings output JSON still contains all expected fields
  • Verify skipped-stages notification still appears on PRs
  • Verify Get-Settings.yml no longer requires pull-requests: write

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions