Skip to content

Fix add_comment and update_* safe outputs ignoring target configuration - #8753

Merged
pelikhan merged 5 commits into
mainfrom
copilot/investigate-comment-bug
Jan 3, 2026
Merged

Fix add_comment and update_* safe outputs ignoring target configuration#8753
pelikhan merged 5 commits into
mainfrom
copilot/investigate-comment-bug

Conversation

CopilotAI commented Jan 3, 2026

Copy link
Copy Markdown
Contributor

The add_comment handler extracted the target configuration but never used it, causing comments to target the wrong PR/issue. In run #20652597117, a workflow triggered by PR #8535 incorrectly commented on unrelated PR #21.

During investigation, found 3 more handlers with the same bug pattern: update_issue.cjs, update_discussion.cjs, and update_pull_request.cjs. All four handlers have been fixed in this PR.

Changes

actions/setup/js/add_comment.cjs

  • Import resolveTarget() from safe_output_helpers.cjs
  • Replace manual context resolution with standardized target handling
  • Preserve discussion-specific logic (separate code path)

actions/setup/js/add_comment.test.cjs (new)

actions/setup/js/update_issue.cjs

  • Import resolveTarget() from safe_output_helpers.cjs
  • Replace manual target resolution with standardized helper
  • Now supports explicit numbers and wildcard mode

actions/setup/js/update_discussion.cjs

  • Import resolveTarget() from safe_output_helpers.cjs
  • Add explicit number target support with improved validation
  • Enhanced error messages

actions/setup/js/update_pull_request.cjs

  • Import resolveTarget() from safe_output_helpers.cjs
  • Replace manual target resolution with standardized helper
  • Now supports explicit numbers and wildcard mode

Before/After

// Before: target config ignoreditemNumber=contextIssue||contextPR;// Always uses context// After: target config respectedconsttargetResult=resolveTarget({targetConfig: commentTarget,// Now uses configitem: item,context: context,itemType: "add_comment",supportsPR: true,});itemNumber=targetResult.number;

Impact

All four handlers now consistently support:

  • "triggering" mode (default) - uses workflow trigger context
  • Explicit number targets (e.g., target: "42") - targets specific issue/PR/discussion
  • Wildcard mode (target: "*") - uses item_number/issue_number/pull_request_number from message

This fix prevents workflows from targeting the wrong PR/issue and provides consistent behavior across all safe-output handlers.

[!WARNING]

Custom agent used: debug-agentic-workflow
Debug and refine agentic workflows using gh-aw CLI tools - analyze logs, audit runs, and improve workflow performance

Custom agent used: debug-agentic-workflow
Debug and refine agentic workflows using gh-aw CLI tools - analyze logs, audit runs, and improve workflow performance

Original prompt

Reference: https://github.com/githubnext/gh-aw/actions/runs/20652597117/job/59299816067#step:29:1

Investigate why the safe output added a comment to the wrong PR (#21 is unrelated to this change). Review the add_comment.cjs logic , this agentic run to determine the root cause of the bug.

Custom agent used: debug-agentic-workflow
Debug and refine agentic workflows using gh-aw CLI tools - analyze logs, audit runs, and improve workflow performance


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

CopilotAIand others added 3 commits January 3, 2026 14:14
- Import resolveTarget helper from safe_output_helpers.cjs
- Replace manual context resolution with proper target handling
- Supports "triggering", explicit numbers, and "*" targets
- Add comprehensive tests for target configuration
- Fix regression: prevents commenting on wrong PR
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
All changes complete and tested:
- Fixed add_comment.cjs to respect target configuration
- Added comprehensive tests with regression coverage
- Recompiled all workflows
- Unit tests pass (44.5s)
- JavaScript tests pass (7/7)
- Integration tests properly skipped (expected)
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot search for other instances of JavaScript code resolving a target incorrectly, always use helper.

CopilotAI changed the title [WIP] Investigate incorrect comment on unrelated PRFix add_comment safe output ignoring target configurationJan 3, 2026
CopilotAI requested a review from pelikhanJanuary 3, 2026 14:31

CopilotAI commented Jan 3, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot search for other instances of JavaScript code resolving a target incorrectly, always use helper.

Found 3 more handlers with incorrect target resolution that should use the helper:

update_issue.cjs (lines 43, 82-99):

  • Extracts updateTarget but only checks "triggering" mode
  • Ignores explicit numbers and "*" wildcard

update_discussion.cjs (lines 76, 104-133):

  • Extracts updateTarget but only checks "triggering" mode
  • Ignores explicit numbers and "*" wildcard

update_pull_request.cjs (lines 78, 108-137):

  • Extracts updateTarget but only checks "triggering" mode
  • Ignores explicit numbers and "*" wildcard

All three should be refactored to use resolveTarget() from safe_output_helpers.cjs like add_comment.cjs now does.

Note: create_pr_review_comment.cjs already has proper target handling (lines 108-150) but uses inline logic instead of the helper - it works correctly but could be refactored for consistency.

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot fix update_*.cjs handlers

…esolveTarget helper
- update_issue.cjs: Import and use resolveTarget for proper target handling
- update_discussion.cjs: Add explicit number target support and improved validation
- update_pull_request.cjs: Import and use resolveTarget for proper target handling
All three handlers now support:
- "triggering" mode (default)
- Explicit number targets (e.g., target: "123")
- Wildcard mode (target: "*" with item_number in message)
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

CopilotAI commented Jan 3, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot fix update_*.cjs handlers

Fixed all three update_*.cjs handlers in commit fd682f3:

update_issue.cjs:

  • Import resolveTarget() helper
  • Replace manual resolution logic with standardized helper
  • Now supports explicit numbers (e.g., target: "42") and wildcard mode (target: "*")

update_discussion.cjs:

  • Import resolveTarget() helper
  • Add explicit number target support
  • Improved error messages and validation

update_pull_request.cjs:

  • Import resolveTarget() helper
  • Replace manual resolution logic with standardized helper
  • Now supports explicit numbers (e.g., target: "123") and wildcard mode (target: "*")

All three handlers now consistently support the same target modes: "triggering" (default), explicit numbers, and wildcard with item_number in the message.

CopilotAI changed the title Fix add_comment safe output ignoring target configurationFix add_comment and update_* safe outputs ignoring target configurationJan 3, 2026
@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot format and lint js

CopilotAI commented Jan 3, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot format and lint js

All JavaScript files are already properly formatted and pass linting:

make fmt-cjs - All files unchanged (already formatted)
make lint-cjs - All matched files use Prettier code style

No changes needed.

@pelikhan
pelikhan marked this pull request as ready for review January 3, 2026 14:56
@pelikhan
pelikhan merged commit 0b6054a into mainJan 3, 2026
125 of 128 checks passed
@pelikhan
pelikhan deleted the copilot/investigate-comment-bug branch January 3, 2026 14:56
Sign up for freeto 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

@pelikhan