Adjust validation order & skip bot triggered response workflows
đī¸ Tracking
đ Objective
We have community users who are seeing workflow failed notifications when comments land on PRs even when it has nothing to do with Claude Code. They are seeing this because we are first executing the check permissions action. The simplest solution today is to validate the comment and then only execute the permission check if the comment does mention @claude.
Also, we have noticed some additional workflow run failures because of bots that add comments. I have added a second validation to ensure bot comments do not trigger an attempt for a Claude Code response. These bots should not ever be mentioning @claude; however, this simple additional validation ensures we don't get into a bot battle endless loop.
đ¸ Screenshots
â° Reminders before review
- Contributor guidelines followed
- All formatters and local linters executed and passed
- Written new unit and / or integration tests where applicable
- Protected functional changes with optionality (feature flags)
- Used internationalization (i18n) for all UI strings
- CI builds passed
- Communicated to DevOps any deployment requirements
- Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team
đĻŽ Reviewer guidelines
- đ (
:+1:) or similar for great changes - đ (
:memo:) or âšī¸ (:information_source:) for notes or general info - â (
:question:) for questions - đ¤ (
:thinking:) or đ (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - đ¨ (
:art:) for suggestions / improvements - â (
:x:) or â ī¸ (:warning:) for more significant problems or concerns needing attention - đą (
:seedling:) or âģī¸ (:recycle:) for future improvements or indications of technical debt - â (
:pick:) for minor or nitpick changes
Claude finished @theMickster's task ââ View job
Code Review Complete
I've reviewed the workflow changes and have the following feedback:
PR Quality Assessment
The PR title and description are clear and well-structured. The objective clearly explains the problem (false-positive workflow failures for community users) and the solution (validation order change + bot detection). Good work on including the context and rationale.
Summary
This PR successfully addresses the stated objectives by reordering validation logic and adding bot detection. The changes are well-implemented with appropriate logging and defensive checks. I've identified a few findings below that warrant attention.
Findings
Finding 1: Shell script uses bash-specific array syntax without explicit bash declaration
Finding 2: Bot validation logic has a potential edge case with undefined event contexts
Finding 3: Validation output messages could be more consistent
Details
Finding 1: Shell script bash requirement (â ī¸)
The check-actor step uses bash array syntax (BLOCKED_BOTS=("claude[bot]" ...)) which requires bash, but GitHub Actions default shell is bash so this works. However, it's a best practice to be explicit when using bash-specific features.
The code is functional as-is since ubuntu runners default to bash, but consider adding shell: bash if you want to be explicit about the requirement.
Current behavior: Works correctly on ubuntu runners Risk level: Low (cosmetic/documentation concern)
Finding 2: Potential undefined variable edge case (đ)
In the check-actor step at line 33:
AUTHOR="${COMMENT_AUTHOR:-${REVIEW_AUTHOR:-${ISSUE_AUTHOR}}}"
This nested parameter expansion will result in an empty string if all three variables are undefined. While this is unlikely given the workflow triggers, the subsequent bot check compares against an empty string, which could theoretically match if a bot username were empty (highly unlikely but theoretically possible).
Question: Have you considered adding an explicit check for empty AUTHOR to make the validation more robust? For example:
if [ -z "$AUTHOR" ]; then
echo "â ī¸ Unable to determine author - skipping workflow"
echo "is_bot=true" >> "$GITHUB_OUTPUT"
exit 0
fi
This would make the intent more explicit and provide better logging if this edge case ever occurs.
Finding 3: Validation message consistency (đ¨)
The validation messages use different patterns:
- Line 77: "â Validation: @claude mentioned in event"
- Line 80: "âī¸ Validation: @claude not mentioned - skipping"
- Line 94: "â Validation passed - workflow will proceed"
- Line 97: "â ī¸ @claude not mentioned - workflow will be skipped"
Consider standardizing the message format across all validation steps for easier log parsing. For example, consistently using the pattern: [Status Icon] [Step Name]: [Outcome] - [Action]
What Works Well
The core logic is sound and achieves the stated objectives:
- Bot detection prevents endless loops
- Early
@claudevalidation prevents unnecessary permission checks - Dependency order (
validationâcheck-runâcomment) is correct - Proper use of job outputs and conditionals
- Good use of emoji for quick log scanning
Workflow Linter Compliance
All workflow linter requirements appear satisfied:
- Permissions explicitly set at workflow and job levels
- Job runner pinned to specific version (ubuntu-24.04)
- External actions pinned to commit hashes with version comments
- Job and workflow names properly capitalized and present
Checkmarx One â Scan Summary & Details â ee58594d-4dbf-42d3-9745-4237a48282a5
Great job! No new security vulnerabilities introduced in this pull request
Subsequent PR will supersede this one.