fix: update-pull-request honors footer: false#17525
Conversation
- Add Footer field to UpdatePullRequestsConfig struct - Propagate footer config in compiler_safe_outputs_config.go - Pass _includeFooter flag through buildPRUpdateData/executePRUpdate - Add update-pull-request to global footer test coverage Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot review the main JSON Schema and docs to make sure footer is documented |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 3023bf3. Updated:
|
|
@copilot review footer implementation and compare to other footer implementation like create-issue. Make sure you use common helpers. |
…ption_helpers Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in c35bb81. Updated
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR adds support for the footer: false configuration option to the update-pull-request safe output, allowing users to suppress the AI-generated footer while preserving XML metadata markers for searchability. This brings consistency with other safe outputs (create-issue, create-pull-request, update-issue, etc.) which already honor this configuration.
Changes:
- Added
Footerfield toUpdatePullRequestsConfigstruct in Go backend with YAML parsing support - Propagated footer configuration from global and local settings to the JavaScript handler
- Updated
buildAIFooterto usegenerateFooterWithMessagesfor consistency with create operations, including installation instructions and metadata - Added workflow-id XML marker when footer is disabled to preserve searchability
- Updated JSON schema and documentation to reflect the new configuration option
- Extended tests to verify global footer configuration affects update-pull-request
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/update_pull_request.go | Added Footer *bool field to config struct and registered it for YAML parsing |
| pkg/workflow/compiler_safe_outputs_config.go | Added footer propagation to handler config using getEffectiveFooter helper |
| pkg/workflow/safe_outputs_footer_test.go | Extended global footer test to verify update-pull-request inherits footer setting |
| actions/setup/js/update_pull_request.cjs | Added footer flag extraction and propagation to updateBody helper |
| actions/setup/js/update_pr_description_helpers.cjs | Enhanced buildAIFooter to use generateFooterWithMessages; added workflow-id marker when footer disabled |
| actions/setup/js/update_pr_description_helpers.test.cjs | Updated test expectation to verify workflow-id marker presence when footer disabled |
| pkg/parser/schemas/main_workflow_schema.json | Added footer property to update-pull-request schema with description |
| docs/src/content/docs/reference/safe-outputs.md | Documented footer: false configuration option in example |
| docs/src/content/docs/reference/footers.md | Added update-pull-request to list of affected safe output types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| includeFooter: false, | ||
| }); | ||
| expect(result).toBe("Replacement"); | ||
| expect(result).toBe("Replacement\n\n<!-- gh-aw-workflow-id: test-workflow -->"); |
There was a problem hiding this comment.
After fixing the bug on line 92 (making the workflow ID marker unconditional), this test expectation will need to be updated. The workflow ID marker should be present in the output regardless of the includeFooter setting. Additionally, tests should be added to verify that the workflow ID marker is present when includeFooter is true (currently only line 461 tests for it when false).
| // When footer is enabled use the full footer (includes install instructions, XML marker, etc.) | ||
| // When footer is disabled still add standalone workflow-id marker for searchability | ||
| const aiFooter = includeFooter ? buildAIFooter(workflowName, runUrl) : ""; | ||
| const workflowIdMarker = !includeFooter && workflowId ? `\n\n${generateWorkflowIdMarker(workflowId)}` : ""; |
There was a problem hiding this comment.
The workflow ID marker should be added unconditionally (when workflowId is present), not just when includeFooter is false. This creates an inconsistency with create_issue and create_pull_request, which always add the workflow-id marker for searchability.
In create_issue.cjs (line 446-448) and create_pull_request.cjs (line 489-491), the workflow-id marker is added unconditionally with the comment "Always add XML markers even when footer is disabled". The same pattern should be followed here.
Change line 92 from:
const workflowIdMarker = !includeFooter && workflowId ? `\n\n${generateWorkflowIdMarker(workflowId)}` : "";
to:
const workflowIdMarker = workflowId ? `\n\n${generateWorkflowIdMarker(workflowId)}` : "";
This ensures the workflow-id marker is always present for searchability, regardless of footer settings.
| const workflowIdMarker = !includeFooter && workflowId ? `\n\n${generateWorkflowIdMarker(workflowId)}` : ""; | |
| const workflowIdMarker = workflowId ? `\n\n${generateWorkflowIdMarker(workflowId)}` : ""; |
update-pull-requestalways appended an AI attribution footer regardless of configuration, unlike all other safe outputs which respectfooter: false.Changes
UpdatePullRequestsConfig(update_pull_request.go): AddedFooter *boolfield and registered it for YAML parsing.compiler_safe_outputs_config.go: AddedgetEffectiveFooterpropagation to theupdate_pull_requesthandler config, enabling both local and global footer suppression.update_pull_request.cjs:buildPRUpdateDatanow stores_includeFooter = config.footer !== false;executePRUpdateextracts it and passes toupdateBody().update_pr_description_helpers.cjs:buildAIFooternow uses the commongenerateFooterWithMessages()helper (consistent withcreate_issueandcreate_pull_request), including installation instructions, XML metadata marker, missing info/blocked-domains sections.updateBodynow also addsgenerateWorkflowIdMarker(workflowId)when footer is disabled, preserving searchability.safe_outputs_footer_test.go: Extended global footer test to coverupdate_pull_request.main_workflow_schema.json): Addedfooterproperty toupdate-pull-request.safe-outputs.md: Addedfooter: falseto theupdate-pull-requestcode example.footers.md: Addedupdate-pull-requestto the list of safe output types affected by globalfooter: false.Usage
Or globally:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.