Skip to content

Solving ADR-006 PR check suggestion error#19

Open
NiallTwomey2 wants to merge 14 commits into
redhat-developer:mainfrom
NiallTwomey2:RHIDP-15121-adr-number-check
Open

Solving ADR-006 PR check suggestion error#19
NiallTwomey2 wants to merge 14 commits into
redhat-developer:mainfrom
NiallTwomey2:RHIDP-15121-adr-number-check

Conversation

@NiallTwomey2

@NiallTwomey2 NiallTwomey2 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

This pull request will update the scripts/suggest-next-adr.sh, to ensure that CI validation (--ci-suggest) evaluates the next suggestion of ADR from the list of already merged ADR's that currently exist inside the 'decisions/' folder on rhdh-adr:main, instead of scanning the PR branch's local 'decisions/' folder to determine the next ADR suggestion to create a pull request with.

Fixes https://redhat.atlassian.net/browse/RHIDP-15384

@NiallTwomey2

Copy link
Copy Markdown
Contributor Author

Hello @rm3l, would you be able to oversee this merge to rhdh-adr:main. I believe that the error you had from the CI check was due to the bash script counting only the ADR files that are locally inside the '/decisions' folder, instead of the ADR files that have already been merged to main. Your file '006-rhdh-local-branching-strategy.md' should be able to merge without the CI check failing.

@rm3l

rm3l commented Jul 10, 2026

Copy link
Copy Markdown
Member

/agentic_review

@rm3l

rm3l commented Jul 10, 2026

Copy link
Copy Markdown
Member

/cc

@openshift-ci openshift-ci Bot requested a review from rm3l July 10, 2026 06:58
Comment thread scripts/suggest-next-adr.sh Outdated
Comment thread scripts/suggest-next-adr.sh Outdated
Comment thread scripts/suggest-next-adr.sh Outdated

@rm3l rm3l left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NiallTwomey2 Looks good, thanks.
But looking at the original error that this PR is supposed to solve, I think a change might be needed in the workflow itself. The error happened in the Comment in PR with suggestion step. So while fixing the script itself will help avoid running this step in most cases, I think the same issue will happen again if this step is executed if the script suggests an ADR number.
A comment is posted for PRs in your personal fork repo, but for security reasons, I don't think a GH workflow triggered on a pull_request event is allowed to post a comment on fork PRs.

@NiallTwomey2

Copy link
Copy Markdown
Contributor Author

@rm3l I understand where the concern is, no suggestion comment for the next ADR was outputted to your PR at all, instead it gave you 'Unhandled error: HttpError: Resource not accessible by integration' because the bot message meant for suggesting the next ADR appears to be blocked within the PR itself. This may be coming from a permissions issue with GitHub, ultimately affecting other PR's that are from forked repositories to upstream (main on rhdh-adr). I didn't acknowledge this issue until now, and will look more into understanding why this is happening. Correct me if I'm wrong of course, this is what I'm understanding what needs to be addressed.

@rm3l

rm3l commented Jul 10, 2026

Copy link
Copy Markdown
Member

@rm3l I understand where the concern is, no suggestion comment for the next ADR was outputted to your PR at all, instead it gave you 'Unhandled error: HttpError: Resource not accessible by integration' because the bot message meant for suggesting the next ADR appears to be blocked within the PR itself. This may be coming from a permissions issue with GitHub, ultimately affecting other PR's that are from forked repositories to upstream (main on rhdh-adr). I didn't acknowledge this issue until now, and will look more into understanding why this is happening. Correct me if I'm wrong of course, this is what I'm understanding what needs to be addressed.

Correct, but note that this is an intentional security protection in GH, AFAICT. Feel free to dig deeper for your own curiosity, but I would suggest not spending too much time on trying to work around this; instead, I would suggest removing this "Comment" step in the workflow (or conditionally skip it only on fork PRs). People can always look at the workflow logs to view the failure reason. A PR comment is nice to have, but not necessary IMO.

@NiallTwomey2

Copy link
Copy Markdown
Contributor Author

The comment step has been removed and the workflow should now output the ADR error and suggested name in the Actions log as an alternative to not being able to see the ADR suggestion within the forked repository due to the comment being blocked. The 'Fail if ADR numbering check step failed' step is what is substituted into the YAML that assigns the output of the errors into the Actions log. Ready for review when you have time next week.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants