Skip to content

Commit f0386ed

Browse files
Copilotpelikhan
andauthored
feat: add merge-base support to update-pull-request safe output
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/340b546b-743d-45b6-ac7e-8ec5735124d3 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent b9e5595 commit f0386ed

11 files changed

Lines changed: 209 additions & 2 deletions

actions/setup/js/safe_outputs_tools.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,10 @@
783783
"enum": ["replace", "append", "prepend"],
784784
"description": "How to update the PR body: 'replace' (default - completely overwrite), 'append' (add to end with separator), or 'prepend' (add to start with separator). Title is always replaced."
785785
},
786+
"merge_base": {
787+
"type": "boolean",
788+
"description": "When true, update the pull request branch with the latest base branch changes before applying other updates. Defaults to false."
789+
},
786790
"pull_request_number": {
787791
"type": ["number", "string"],
788792
"description": "Pull request number to update. This is the numeric ID from the GitHub URL (e.g., 234 in github.com/owner/repo/pull/234). Required when the workflow target is '*' (any PR)."

actions/setup/js/types/safe-outputs.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,8 @@ interface UpdatePullRequestItem extends BaseSafeOutputItem {
235235
body?: string;
236236
/** Update operation for body: 'replace' (default), 'append', or 'prepend' */
237237
operation?: "replace" | "append" | "prepend";
238+
/** When true, merges the latest base branch changes into the pull request branch before other updates */
239+
merge_base?: boolean;
238240
/** Optional pull request number for target "*" */
239241
pull_request_number?: number | string;
240242
/** Whether the PR should be a draft (true) or ready for review (false) */

actions/setup/js/update_pull_request.cjs

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,17 @@ async function executePRUpdate(github, context, prNumber, updateData) {
3232

3333
// Remove internal fields
3434
const { _operation, _rawBody, _includeFooter, _workflowRepo, ...apiData } = updateData;
35+
const mergeBase = apiData.merge_base === true;
36+
delete apiData.merge_base;
37+
38+
if (mergeBase) {
39+
core.info(`Updating pull request #${prNumber} branch with base branch changes`);
40+
await github.rest.pulls.updateBranch({
41+
owner: context.repo.owner,
42+
repo: context.repo.repo,
43+
pull_number: prNumber,
44+
});
45+
}
3546

3647
// If we have a body, process it with the appropriate operation
3748
if (rawBody !== undefined) {
@@ -77,6 +88,15 @@ async function executePRUpdate(github, context, prNumber, updateData) {
7788
core.info(`Will update body (length: ${apiData.body.length})`);
7889
}
7990

91+
if (Object.keys(apiData).length === 0) {
92+
const { data: pr } = await github.rest.pulls.get({
93+
owner: context.repo.owner,
94+
repo: context.repo.repo,
95+
pull_number: prNumber,
96+
});
97+
return pr;
98+
}
99+
80100
const { data: pr } = await github.rest.pulls.update({
81101
owner: context.repo.owner,
82102
repo: context.repo.repo,
@@ -141,6 +161,12 @@ function buildPRUpdateData(item, config) {
141161
hasUpdates = true;
142162
}
143163

164+
const mergeBase = item.merge_base !== undefined ? item.merge_base === true : config.merge_base === true;
165+
if (mergeBase) {
166+
updateData.merge_base = true;
167+
hasUpdates = true;
168+
}
169+
144170
if (!hasUpdates) {
145171
return {
146172
success: true,
@@ -181,7 +207,8 @@ const main = createUpdateHandlerFactory({
181207
additionalConfig: {
182208
allow_title: true,
183209
allow_body: true,
210+
merge_base: false,
184211
},
185212
});
186213

187-
module.exports = { main };
214+
module.exports = { main, buildPRUpdateData };

actions/setup/js/update_pull_request.test.cjs

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const mockGithub = {
2323
pulls: {
2424
get: vi.fn(),
2525
update: vi.fn(),
26+
updateBranch: vi.fn(),
2627
},
2728
},
2829
};
@@ -81,6 +82,11 @@ describe("update_pull_request.cjs - executePRUpdate function", () => {
8182
html_url: "https://github.com/testowner/testrepo/pull/100",
8283
},
8384
});
85+
mockGithub.rest.pulls.updateBranch.mockResolvedValue({
86+
data: {
87+
message: "Branch updated",
88+
},
89+
});
8490
});
8591

8692
describe("Replace operation", () => {
@@ -738,3 +744,84 @@ describe("update_pull_request.cjs - executePRUpdate function", () => {
738744
});
739745
});
740746
});
747+
748+
describe("update_pull_request.cjs - merge_base behavior", () => {
749+
beforeEach(async () => {
750+
vi.clearAllMocks();
751+
vi.resetModules();
752+
753+
updatePRModule = await import("./update_pull_request.cjs");
754+
755+
mockGithub.rest.pulls.get.mockResolvedValue({
756+
data: {
757+
number: 100,
758+
title: "Test PR",
759+
body: "Original body content",
760+
html_url: "https://github.com/testowner/testrepo/pull/100",
761+
},
762+
});
763+
764+
mockGithub.rest.pulls.update.mockResolvedValue({
765+
data: {
766+
number: 100,
767+
title: "Updated PR",
768+
body: "Original body content",
769+
html_url: "https://github.com/testowner/testrepo/pull/100",
770+
},
771+
});
772+
773+
mockGithub.rest.pulls.updateBranch.mockResolvedValue({
774+
data: {
775+
message: "Branch updated",
776+
},
777+
});
778+
});
779+
780+
it("should include merge_base when item requests it", () => {
781+
const result = updatePRModule.buildPRUpdateData({ merge_base: true }, {});
782+
783+
expect(result.success).toBe(true);
784+
expect(result.data.merge_base).toBe(true);
785+
});
786+
787+
it("should inherit merge_base from config when item does not set it", () => {
788+
const result = updatePRModule.buildPRUpdateData({}, { merge_base: true });
789+
790+
expect(result.success).toBe(true);
791+
expect(result.data.merge_base).toBe(true);
792+
});
793+
794+
it("should call updateBranch when merge_base is enabled and no other fields are updated", async () => {
795+
const handlerFactory = await updatePRModule.main();
796+
const handler = await handlerFactory({ merge_base: true });
797+
798+
const result = await handler({ pull_request_number: 100 });
799+
800+
expect(result.success).toBe(true);
801+
expect(mockGithub.rest.pulls.updateBranch).toHaveBeenCalledWith({
802+
owner: "testowner",
803+
repo: "testrepo",
804+
pull_number: 100,
805+
});
806+
expect(mockGithub.rest.pulls.update).not.toHaveBeenCalled();
807+
});
808+
809+
it("should call updateBranch before pulls.update when merge_base and title update are both requested", async () => {
810+
const handlerFactory = await updatePRModule.main();
811+
const handler = await handlerFactory({});
812+
813+
await handler({
814+
pull_request_number: 100,
815+
title: "Updated PR",
816+
merge_base: true,
817+
});
818+
819+
expect(mockGithub.rest.pulls.updateBranch).toHaveBeenCalled();
820+
expect(mockGithub.rest.pulls.update).toHaveBeenCalledWith({
821+
owner: "testowner",
822+
repo: "testrepo",
823+
pull_number: 100,
824+
title: "Updated PR",
825+
});
826+
});
827+
});

pkg/parser/schemas/main_workflow_schema.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6976,6 +6976,10 @@
69766976
"type": "boolean",
69776977
"description": "Allow updating pull request body - defaults to true, set to false to disable"
69786978
},
6979+
"merge-base": {
6980+
"type": "boolean",
6981+
"description": "When true, update the pull request branch with the latest base branch changes before applying other updates. Defaults to false."
6982+
},
69796983
"operation": {
69806984
"type": "string",
69816985
"description": "Default operation for body updates: 'append' (add to end), 'prepend' (add to start), or 'replace' (overwrite completely). Defaults to 'replace' if not specified.",

pkg/workflow/compiler_safe_outputs_config_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,6 +1203,65 @@ func TestHandlerConfigUpdateFields(t *testing.T) {
12031203
}
12041204
}
12051205

1206+
func TestUpdatePullRequestMergeBaseHandlerConfig(t *testing.T) {
1207+
tests := []struct {
1208+
name string
1209+
mergeBase *bool
1210+
expected bool
1211+
}{
1212+
{
1213+
name: "defaults merge_base to false",
1214+
mergeBase: nil,
1215+
expected: false,
1216+
},
1217+
{
1218+
name: "sets merge_base true when configured",
1219+
mergeBase: testBoolPtr(true),
1220+
expected: true,
1221+
},
1222+
}
1223+
1224+
for _, tt := range tests {
1225+
t.Run(tt.name, func(t *testing.T) {
1226+
compiler := NewCompiler()
1227+
1228+
workflowData := &WorkflowData{
1229+
Name: "Test Workflow",
1230+
SafeOutputs: &SafeOutputsConfig{
1231+
UpdatePullRequests: &UpdatePullRequestsConfig{
1232+
MergeBase: tt.mergeBase,
1233+
},
1234+
},
1235+
}
1236+
1237+
var steps []string
1238+
compiler.addHandlerManagerConfigEnvVar(&steps, workflowData)
1239+
1240+
for _, step := range steps {
1241+
if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") {
1242+
parts := strings.Split(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ")
1243+
if len(parts) == 2 {
1244+
jsonStr := strings.TrimSpace(parts[1])
1245+
jsonStr = strings.Trim(jsonStr, "\"")
1246+
jsonStr = strings.ReplaceAll(jsonStr, "\\\"", "\"")
1247+
1248+
var config map[string]map[string]any
1249+
err := json.Unmarshal([]byte(jsonStr), &config)
1250+
require.NoError(t, err)
1251+
1252+
updatePRConfig, ok := config["update_pull_request"]
1253+
require.True(t, ok, "Expected update_pull_request config")
1254+
1255+
mergeBaseValue, ok := updatePRConfig["merge_base"]
1256+
require.True(t, ok, "Expected merge_base key in update_pull_request config")
1257+
assert.Equal(t, tt.expected, mergeBaseValue)
1258+
}
1259+
}
1260+
}
1261+
})
1262+
}
1263+
}
1264+
12061265
// TestEmptySafeOutputsConfig tests behavior with no safe outputs
12071266
func TestEmptySafeOutputsConfig(t *testing.T) {
12081267
compiler := NewCompiler()

pkg/workflow/compiler_safe_outputs_handlers.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,7 @@ var handlerRegistry = map[string]handlerBuilder{
437437
AddIfNotEmpty("target", c.Target).
438438
AddBoolPtrOrDefault("allow_title", c.Title, true).
439439
AddBoolPtrOrDefault("allow_body", c.Body, true).
440+
AddBoolPtrOrDefault("merge_base", c.MergeBase, false).
440441
AddStringPtr("default_operation", c.Operation).
441442
AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)).
442443
AddIfNotEmpty("target-repo", c.TargetRepoSlug).

pkg/workflow/js/safe_outputs_tools.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -935,6 +935,10 @@
935935
],
936936
"description": "How to update the PR body: 'replace' (default - completely overwrite), 'append' (add to end with separator), or 'prepend' (add to start with separator). Title is always replaced."
937937
},
938+
"merge_base": {
939+
"type": "boolean",
940+
"description": "When true, update the pull request branch with the latest base branch changes before applying other updates. Defaults to false."
941+
},
938942
"pull_request_number": {
939943
"type": [
940944
"number",

pkg/workflow/safe_output_validation_config_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,11 +188,27 @@ func TestUpdateDiscussionValidationConfig(t *testing.T) {
188188
}
189189
}
190190

191+
func TestUpdatePullRequestValidationConfig(t *testing.T) {
192+
config, ok := ValidationConfig["update_pull_request"]
193+
if !ok {
194+
t.Fatal("update_pull_request not found in ValidationConfig")
195+
}
196+
197+
if config.CustomValidation != "requiresOneOf:title,body,merge_base" {
198+
t.Errorf("update_pull_request customValidation = %q, want %q", config.CustomValidation, "requiresOneOf:title,body,merge_base")
199+
}
200+
201+
if _, ok := config.Fields["merge_base"]; !ok {
202+
t.Error("update_pull_request Fields is missing the 'merge_base' field")
203+
}
204+
}
205+
191206
func TestValidationConfigConsistency(t *testing.T) {
192207
// Verify that all types with customValidation have valid validation rules
193208
validCustomValidations := map[string]bool{
194209
"requiresOneOf:status,title,body": true,
195210
"requiresOneOf:title,body": true,
211+
"requiresOneOf:title,body,merge_base": true,
196212
"requiresOneOf:title,body,labels": true,
197213
"requiresOneOf:issue_number,pull_number": true,
198214
"requiresOneOf:reviewers,team_reviewers": true,

pkg/workflow/safe_outputs_validation_config.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,11 +154,12 @@ var ValidationConfig = map[string]TypeValidationConfig{
154154
},
155155
"update_pull_request": {
156156
DefaultMax: 1,
157-
CustomValidation: "requiresOneOf:title,body",
157+
CustomValidation: "requiresOneOf:title,body,merge_base",
158158
Fields: map[string]FieldValidation{
159159
"title": {Type: "string", Sanitize: true, MaxLength: 256},
160160
"body": {Type: "string", Sanitize: true, MaxLength: MaxBodyLength},
161161
"operation": {Type: "string", Enum: []string{"replace", "append", "prepend"}},
162+
"merge_base": {Type: "boolean"},
162163
"draft": {Type: "boolean"},
163164
"pull_request_number": {IssueOrPRNumber: true},
164165
"repo": {Type: "string", MaxLength: 256}, // Optional: target repository in format "owner/repo"

0 commit comments

Comments
 (0)