Skip to content

Allow approval comments to include reasons - #234

Open
CAOShurong wants to merge 3 commits into
trstringer:mainfrom
CAOShurong:codex/21-comment-reasons
Open

CAOShurong wants to merge 3 commits into
trstringer:mainfrom
CAOShurong:codex/21-comment-reasons

Conversation

@CAOShurong

@CAOShurong CAOShurong commented Aug 15, 2026 •

Copy link
Copy Markdown

Summary

  • Add an opt-in allow-comment-reasons input.
  • When enabled, only the first line determines approval or denial; later lines can explain the decision without introducing ambiguous keywords.
  • Keep the input disabled by default, preserving v1's exact-comment behavior.

Closes #21.

Requested test coverage

This update addresses the request to test both values explicitly. The same 11 parser scenarios each run with false and true (22 cases), covering reasons, CRLF, exact keywords, later/empty first lines, same-line prose, unauthorized users, and distinct versus duplicate approvers.

Four httptest polling cases cover approval and denial in both modes. They verify whether the reason-bearing comment completes immediately or waits for an exact keyword, as well as the terminal comment, issue closure and exit code. This revision adds tests only; a normal merge retains the two current upstream polling fixes.

Current-revision verification

On Go 1.26.7 / Windows amd64, including a clean archive byte-matched to commit 0d4edbd3a233d304a3e91230c88e70bc64e872ae:

  • go test -json -count=20 -p=2 ./...: all 76 leaf cases pass in each repetition.
  • go vet -p=2 ./... and go build -p=2 -buildvcs=false -trimpath ... .: pass.
  • CI's exact golangci-lint v2.10.1 run -v: zero issues.
  • Gofmt/diff checks and the secret, EOF and whitespace hooks: pass.
  • The compiled program passes 16 localhost fake-GitHub API scenarios, checking outputs, exits and requests: both modes, omitted/empty defaults, invalid input without issue writes, unauthorized/exact comments, denial continuation, custom words, distinct approvals, and an API error.
  • Four isolated fixed-boolean mutations of parsing/polling each fail the appropriate newly added tests, demonstrating that wrong-mode wiring is detected.

The fork CI run also passed on exact head 0d4edbd3a233d304a3e91230c88e70bc64e872ae: the unchanged upstream workflow's Linux Docker build, full Go tests (including the explicit false/true cases), and Docker-based golangci-lint v2.10.1 with zero issues. The completed run, job steps and logs were checked. This is fork execution, not upstream maintainer approval.

Limitations: Windows is not a supported Action runner; the local program/API checks are synthetic, not deployed workflow proof. Race, ARM and a real approval/deployment workflow are not tested. The pinned pre-commit lint v1.52.2 panics with unsupported version: 2 on both this revision and the untouched pre-test baseline; the complete configured hook set is not claimed green. Its initial environment-install timeout is also retained. The old August run is not evidence for this revision. No container image or release is published by this update.

Provenance correction

Prepared with OpenAI Codex: AI-generated code, tests and PR text. The previous sentence suggesting personal review was not substantiated by the available records and has been removed. No personal or independent human review, production adoption, or maintainer acceptance is asserted.

Keep exact-comment behavior as the default and add an opt-in first-line decision mode for explanatory follow-up text.

Assisted-by: OpenAI Codex
@snskArora snskArora added the enhancement New feature or request label Aug 28, 2026
@snskArora

Copy link
Copy Markdown
Collaborator

Hi @CAOShurong

Can you please test these changes and produce tests with both possible values for your new variable?

…asons

Signed-off-by: CAOShurong <170531907+CAOShurong@users.noreply.github.com>
Signed-off-by: CAOShurong <170531907+CAOShurong@users.noreply.github.com>
@CAOShurong

Copy link
Copy Markdown
Author

Added the requested explicit coverage in 0d4edbd3: the same 11 parser scenarios run with both false and true, and four HTTP polling tests exercise approval/denial in both modes, including poll counts, closing comments, issue closure and exit codes. Default exact-comment behavior remains unchanged.

The full suite passed 20 repetitions on the byte-verified committed source. The exact-head fork CI also passed the upstream Linux Docker build, tests and v2.10.1 lint (zero issues). No production code was added in this test revision; the merge preserves the current upstream polling fixes.

Prepared with OpenAI Codex (AI-generated code/tests/text). The PR description now corrects the earlier unsupported personal-review wording and records the local pinned-lint compatibility failure and untested race/ARM/deployment paths. Fork CI success is not maintainer approval or deployment evidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow additional information on the comment that approves/denies

2 participants