[zakisk]

📝 Description of the Change

Add a support-edited-comments ConfigMap setting (default: false) that allows Pipelines-as-Code to process edited issue and commit comment events on GitHub. When enabled, editing a comment containing a GitOps command (e.g. /test, /retest) or comment matching on-comment triggers the command as if it were newly created.

This is gated behind a setting because re-triggering on every comment edit could cause unexpected processes and logs.

🔗 Linked GitHub Issue

Fixes #

🧪 Testing Strategy

• Unit tests
• Integration tests
• End-to-end tests
• Manual testing
• Not Applicable

🤖 AI Assistance

AI assistance can be used for various tasks, such as code generation,
documentation, or testing.

Please indicate whether you have used AI assistance
for this PR and provide details if applicable.

• I have not used any AI assistance for this PR.
• I have used AI assistance for this PR.

Important

Slop will be simply rejected, if you are using AI assistance you need to make sure you
understand the code generated and that it meets the project's standards. you
need at least know how to run the code and deploy it (if needed). See
startpaac to make it easy
to deploy and test your code changes.

If the majority of the code in this PR was generated by an AI, please add a Co-authored-by trailer to your commit message.
For example:

Co-authored-by: Claude noreply@anthropic.com

✅ Submitter Checklist

• 📝 My commit messages are clear, informative, and follow the project's How to write a git commit message guide. The Gitlint linter ensures in CI it's properly validated
• ✨ I have ensured my commit message prefix (e.g., fix:, feat:) matches the "Type of Change" I selected above.
• ♽ I have run make test and make lint locally to check for and fix any
  issues. For an efficient workflow, I have considered installing
  pre-commit and running pre-commit install to
  automate these checks.
• 📖 I have added or updated documentation for any user-facing changes.
• 🧪 I have added sufficient unit tests for my code changes.
• 🎁 I have added end-to-end tests where feasible. See README for more details.
• 🔎 I have addressed any CI test flakiness or provided a clear reason to bypass it.
• If adding a provider feature, I have filled in the following and updated the provider documentation:
  • GitHub App
  • GitHub Webhook
  • Gitea/Forgejo
  • GitLab
  • Bitbucket Cloud
  • Bitbucket Data Center

[pipelines-as-code]

Paco Review

The PR adds support for processing edited GitHub comments as GitOps commands, gated behind a new support-edited-comments ConfigMap setting (default false). It modifies the adapter to pass PacOpts into the provider, updates the GitHub detect logic to accept edited action on issue_comment and commit_comment events when enabled, adds unit tests and E2E tests, and documents the feature. The E2E test file lost its //go:build e2e constraint, which would cause those tests to run outside the E2E environment.

Paco could not check everything:

• Could not load 5 files at PR head 57a8adf and base a53fa58 because they were too large, looked like credentials, or could not be read.

[HIGH] test/github_pullrequest_test_comment_test.go (before line 1): Claim. The //go:build e2e build constraint was deleted from the E2E test file. Without it, the E2E tests in this file will be compiled and run during normal go test ./... invocations, which will fail on any machine or CI job that lacks the full E2E infrastructure (GitHub App credentials, running cluster, etc.). Every other E2E test file in the test/ directory retains this build tag.

Trigger. Run go test ./test/... without the e2e build tag. The new TestGithubGHEPullRequestTestEditedComment and TestGithubGHEPullRequestOnCommentEditedComment tests will be included in the build and fail at runtime because no E2E environment is available.

Impact. Unit test CI runs break or include tests that immediately fail because they depend on external E2E infrastructure that is not present.

Fix. Restore the //go:build e2e line at the top of the file, before the package test declaration, matching all other E2E test files in the directory.

_{Reviewed commit: 57a8adf. 1 verified findings; 1 summary-only.}

[pipelines-as-code]

Paco inline comments -- see the Paco Review summary comment for the overview.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.98%. Comparing base (a53fa58) to head (57a8adf).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3051   +/-   ##
=======================================
  Coverage   91.98%   91.98%           
=======================================
  Files         167      167           
  Lines       12973    12976    +3     
=======================================
+ Hits        11933    11936    +3     
  Misses       1039     1039           
  Partials        1        1           

Flag	Coverage Δ
unit-tests	91.98% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

🟡 Changes recommended

Edited commands can select the wrong authorization policy, and test compilation, E2E gating, and ConfigMap selection need correction.

6 open issues

• Apply command authorization policy consistently to edited issue comments · New
• Apply command authorization policy consistently to edited commit comments · New
• Initialize promoted SupportEditedComments field after PacOpts creation · New
• Restore E2E build constraint for live GitHub and Kubernetes tests · New
• Use configured ConfigMap name for edited-comment test setup · New
• Use configured ConfigMap name in on-comment test setup · New

What changed in this PR

Adds opt-in processing of edited GitHub comments so users can correct GitOps commands or trigger on-comment pipelines without posting a new comment.

Changes:

• Adds the disabled-by-default support-edited-comments setting and passes it into GitHub event detection.
• Adds detection tests and end-to-end scenarios for edited comments.
• Documents configuration, provider support, and retriggering behavior.

File	Description
test/​github_pullrequest_test_comment_test.go	Adds edited-comment E2E scenarios and removes the build constraint.
pkg/​provider/​github/​detect.go	Accepts edited issue and commit comments when enabled.
pkg/​provider/​github/​detect_test.go	Adds enabled and disabled detection cases.
pkg/​params/​settings/​config.go	Defines the new boolean setting.
pkg/​params/​settings/​config_test.go	Updates expected configuration values.
pkg/​adapter/​adapter.go	Supplies settings before GitHub event detection.
pkg/​adapter/​adapter_test.go	Updates detection calls to pass options.
docs/​content/​docs/​guides/​gitops-commands/​_index.md	Explains edited-command support and retriggering risks.
docs/​content/​docs/​api/​configmap.md	Documents the setting and its default.
config/​302-pac-configmap.yaml	Adds the disabled setting to the deployment ConfigMap.

_{Review effort: Balanced}

[theakshaypant]

How would ACL work for edited comments?
For example, I can edit a comment from someone outside the org

[zakisk]

@theakshaypant yeah good point!, at the moment it's getting the login of the person editing the comment but I think it should always be the author of the comment instead of the last one person edited it. at the moment if an unauthorized user makes a comment e.g. /ok-to-testo (it has typo) and repo admin edits it to /ok-to-test then it will trigger the CI. should we always use the id of comment author? (I am inclined towards using id of comment author)
cc: @chmouel

[theakshaypant]

I think the distinction of author's permissions, not the editor, being used should be documented given how this is worded "... as if it were a new comment"

[theakshaypant]

Consider the scenario you mentioned in an earlier comment, comment author (without permissions on the repo) has a typo /ok-ot-test which is corrected by a repo maintainer, nothing will happen.

[theakshaypant]

I think we can hold off on this until the original query is addressed

[zakisk]

nothing will happen.

No, it will trigger PipelineRuns

[theakshaypant]

@theakshaypant yeah good point!, at the moment it's getting the login of the person editing the comment but I think it should always be the author of the comment instead of the last one person edited it. at the moment if an unauthorized user makes a comment e.g. /ok-to-testo (it has typo) and repo admin edits it to /ok-to-test then it will trigger the CI. should we always use the id of comment author? (I am inclined towards using id of comment author) cc: @chmouel

Hmm, I think it should be the person who edits the comment since that is where the event originates. Similar to rerun checks does not take the author PR but the actor who clicks rerun.

I do like that we are adding this behind a flag in the configmap.

I am a bit concerned about scenarios where pac itself edits comments (webhook mode with comment_strategy update for example), would that cause a burn (and how much) of the rate limit?

[zakisk]

I think it should be the person who edits the comment since that is where the event originates.

yeah but it would confuse someone that CI is being triggered on unauthorized user's comment.

would that cause a burn (and how much) of the rate limit?

on every issue_commnet we call getPullRequest so yeah on every comment there is one API call. I will document this point as well that if edit comment process is enabled it would increase API call consumption.

[zakisk]

I will document this point as well that if edit comment process is enabled it would increase API call consumption.

@theakshaypant documented it

[theakshaypant]

I think it should be the person who edits the comment since that is where the event originates.

yeah but it would confuse someone that CI is being triggered on unauthorized user's comment.

That is true, so changing it to the author only would solve this confusion.
And the goal to correct any typos for any commands is still achieved.

[theakshaypant]

Nice, thanks!

[chmouel]

/paco review