Skip to content

Commit ef236d5

Browse files
zwickzwick
committed
Carry batch refs with resolved items
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d7dc302d-e6f2-41e9-a2c8-ed598de47067
1 parent 93b824f commit ef236d5

2 files changed

Lines changed: 57 additions & 43 deletions

File tree

‎pkg/github/projects_batch.go‎

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,19 @@ type batchItemError struct {
5050

5151
type resolvedBatchItem struct {
5252
index int
53+
ref map[string]any
5354
nodeID string
5455
fullDatabaseID int64
5556
}
5657

58+
type batchWriteOperation struct {
59+
gqlClient *githubv4.Client
60+
kind batchMutationKind
61+
projectID githubv4.ID
62+
fieldID githubv4.ID
63+
value githubv4.ProjectV2FieldValue
64+
}
65+
5766
func updateProjectItemsBatch(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, args map[string]any) (*mcp.CallToolResult, any, error) {
5867
rawItems, exists := args["items"]
5968
if !exists {
@@ -158,10 +167,16 @@ func updateProjectItemsBatch(ctx context.Context, client *github.Client, gqlClie
158167
}
159168

160169
seenTargets[nodeID] = i
161-
work = append(work, resolvedBatchItem{index: i, nodeID: nodeID, fullDatabaseID: fullDatabaseID})
170+
work = append(work, resolvedBatchItem{index: i, ref: p.ref, nodeID: nodeID, fullDatabaseID: fullDatabaseID})
162171
}
163172

164-
executeBatchWrites(ctx, gqlClient, kind, projectID, githubv4.ID(field.NodeID), value, work, results, parsed)
173+
executeBatchWrites(ctx, batchWriteOperation{
174+
gqlClient: gqlClient,
175+
kind: kind,
176+
projectID: projectID,
177+
fieldID: githubv4.ID(field.NodeID),
178+
value: value,
179+
}, work, results)
165180

166181
return newUpdateProjectItemsResult(results)
167182
}
@@ -230,10 +245,10 @@ func resolveItemReference(p parsedBatchItem, itemIDLookups map[int64]itemLookupR
230245

231246
// Transport, cancellation, or incomplete-data ambiguity stops later chunks;
232247
// GraphQL response errors do not because populated aliases still confirm writes.
233-
func executeBatchWrites(ctx context.Context, gqlClient *githubv4.Client, kind batchMutationKind, projectID, fieldID githubv4.ID, value githubv4.ProjectV2FieldValue, items []resolvedBatchItem, results []batchItemResult, parsed []parsedBatchItem) {
248+
func executeBatchWrites(ctx context.Context, operation batchWriteOperation, items []resolvedBatchItem, results []batchItemResult) {
234249
for start := 0; start < len(items); start += batchMutationWireChunkSize {
235250
if ctx.Err() != nil {
236-
markChunkUnknown(items[start:], results, parsed, ctx.Err())
251+
markChunkUnknown(items[start:], results, ctx.Err())
237252
return
238253
}
239254

@@ -242,23 +257,23 @@ func executeBatchWrites(ctx context.Context, gqlClient *githubv4.Client, kind ba
242257

243258
inputs := make([]githubv4.Input, len(chunk))
244259
for i, item := range chunk {
245-
if kind == batchMutationClear {
260+
if operation.kind == batchMutationClear {
246261
inputs[i] = githubv4.ClearProjectV2ItemFieldValueInput{
247-
ProjectID: projectID,
262+
ProjectID: operation.projectID,
248263
ItemID: githubv4.ID(item.nodeID),
249-
FieldID: fieldID,
264+
FieldID: operation.fieldID,
250265
}
251266
} else {
252267
inputs[i] = githubv4.UpdateProjectV2ItemFieldValueInput{
253-
ProjectID: projectID,
268+
ProjectID: operation.projectID,
254269
ItemID: githubv4.ID(item.nodeID),
255-
FieldID: fieldID,
256-
Value: value,
270+
FieldID: operation.fieldID,
271+
Value: operation.value,
257272
}
258273
}
259274
}
260275

261-
outcomes, mutateErr := executeAliasedMutation(ctx, gqlClient, kind, inputs)
276+
outcomes, mutateErr := executeAliasedMutation(ctx, operation.gqlClient, operation.kind, inputs)
262277

263278
populated := 0
264279
for i, oc := range outcomes {
@@ -267,7 +282,7 @@ func executeBatchWrites(ctx context.Context, gqlClient *githubv4.Client, kind ba
267282
results[chunk[i].index] = batchItemResult{
268283
Index: chunk[i].index,
269284
Status: batchItemSucceeded,
270-
Ref: parsed[chunk[i].index].ref,
285+
Ref: chunk[i].ref,
271286
Item: &batchItemIdentity{
272287
NodeID: oc.NodeID,
273288
FullDatabaseID: oc.FullDatabaseID,
@@ -278,45 +293,45 @@ func executeBatchWrites(ctx context.Context, gqlClient *githubv4.Client, kind ba
278293
}
279294

280295
if isGraphQLResponseError(mutateErr) {
281-
markUnpopulatedUnknown(chunk, outcomes, results, parsed, mutateErr)
296+
markUnpopulatedUnknown(chunk, outcomes, results, mutateErr)
282297
continue
283298
}
284299

285300
if mutateErr != nil {
286-
markChunkUnknown(items[start:], results, parsed, mutateErr)
301+
markChunkUnknown(items[start:], results, mutateErr)
287302
return
288303
}
289304

290305
if populated != len(chunk) {
291-
markChunkUnknown(items[start:], results, parsed, fmt.Errorf("mutation response did not include every item"))
306+
markChunkUnknown(items[start:], results, fmt.Errorf("mutation response did not include every item"))
292307
return
293308
}
294309
}
295310
}
296311

297-
func markUnpopulatedUnknown(chunk []resolvedBatchItem, outcomes []mutationAliasOutcome, results []batchItemResult, parsed []parsedBatchItem, err error) {
312+
func markUnpopulatedUnknown(chunk []resolvedBatchItem, outcomes []mutationAliasOutcome, results []batchItemResult, err error) {
298313
for i, oc := range outcomes {
299314
if oc.Populated {
300315
continue
301316
}
302317
results[chunk[i].index] = batchItemResult{
303318
Index: chunk[i].index,
304319
Status: batchItemUnknown,
305-
Ref: parsed[chunk[i].index].ref,
320+
Ref: chunk[i].ref,
306321
Error: &batchItemError{Code: "mutation_unconfirmed", Message: err.Error()},
307322
}
308323
}
309324
}
310325

311-
func markChunkUnknown(chunk []resolvedBatchItem, results []batchItemResult, parsed []parsedBatchItem, err error) {
326+
func markChunkUnknown(chunk []resolvedBatchItem, results []batchItemResult, err error) {
312327
for _, item := range chunk {
313328
if results[item.index].Status == batchItemSucceeded {
314329
continue
315330
}
316331
results[item.index] = batchItemResult{
317332
Index: item.index,
318333
Status: batchItemUnknown,
319-
Ref: parsed[item.index].ref,
334+
Ref: item.ref,
320335
Error: &batchItemError{Code: "mutation_unconfirmed", Message: err.Error()},
321336
}
322337
}

‎pkg/github/projects_batch_mutation_test.go‎

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -265,9 +265,9 @@ func Test_ExecuteBatchWrites_AllAliasGraphQLErrorContinues(t *testing.T) {
265265
},
266266
},
267267
}
268-
items, parsed, results := batchItemsOfSize(21)
268+
items, results := batchItemsOfSize(21)
269269

270-
executeTestBatchWrites(t.Context(), newTestGQLClient(transport), items, results, parsed)
270+
executeTestBatchWrites(t.Context(), newTestGQLClient(transport), items, results)
271271

272272
assert.Len(t, transport.calls, 2)
273273
for i := range 20 {
@@ -290,12 +290,14 @@ func Test_ExecuteBatchWrites_PartialGraphQLErrorPreservesSuccess(t *testing.T) {
290290
},
291291
},
292292
}
293-
items, parsed, results := batchItemsOfSize(2)
293+
items, results := batchItemsOfSize(2)
294294

295-
executeTestBatchWrites(t.Context(), newTestGQLClient(transport), items, results, parsed)
295+
executeTestBatchWrites(t.Context(), newTestGQLClient(transport), items, results)
296296

297297
assert.Equal(t, batchItemSucceeded, results[0].Status)
298+
assert.Equal(t, items[0].ref, results[0].Ref)
298299
assert.Equal(t, batchItemUnknown, results[1].Status)
300+
assert.Equal(t, items[1].ref, results[1].Ref)
299301
}
300302

301303
func Test_ExecuteBatchWrites_AmbiguousSuccessResponseAborts(t *testing.T) {
@@ -336,9 +338,9 @@ func Test_ExecuteBatchWrites_AmbiguousSuccessResponseAborts(t *testing.T) {
336338
},
337339
},
338340
}
339-
items, parsed, results := batchItemsOfSize(21)
341+
items, results := batchItemsOfSize(21)
340342

341-
executeTestBatchWrites(t.Context(), newTestGQLClient(transport), items, results, parsed)
343+
executeTestBatchWrites(t.Context(), newTestGQLClient(transport), items, results)
342344

343345
assert.Len(t, transport.calls, 1)
344346
for i, result := range results {
@@ -354,9 +356,9 @@ func Test_ExecuteBatchWrites_AmbiguousSuccessResponseAborts(t *testing.T) {
354356

355357
func Test_ExecuteBatchWrites_TransportTimeoutAborts(t *testing.T) {
356358
transport := &errorGraphQLTransport{err: context.DeadlineExceeded}
357-
items, parsed, results := batchItemsOfSize(21)
359+
items, results := batchItemsOfSize(21)
358360

359-
executeTestBatchWrites(t.Context(), newTestGQLClient(transport), items, results, parsed)
361+
executeTestBatchWrites(t.Context(), newTestGQLClient(transport), items, results)
360362

361363
assert.Equal(t, 1, transport.calls)
362364
for _, result := range results {
@@ -368,45 +370,42 @@ func Test_ExecuteBatchWrites_CanceledContextSkipsWrites(t *testing.T) {
368370
ctx, cancel := context.WithCancel(t.Context())
369371
cancel()
370372
transport := &sequencedGraphQLTransport{t: t}
371-
items, parsed, results := batchItemsOfSize(21)
373+
items, results := batchItemsOfSize(21)
372374

373-
executeTestBatchWrites(ctx, newTestGQLClient(transport), items, results, parsed)
375+
executeTestBatchWrites(ctx, newTestGQLClient(transport), items, results)
374376

375377
assert.Empty(t, transport.calls)
376378
for _, result := range results {
377379
assert.Equal(t, batchItemUnknown, result.Status)
378380
}
379381
}
380382

381-
func executeTestBatchWrites(ctx context.Context, gqlClient *githubv4.Client, items []resolvedBatchItem, results []batchItemResult, parsed []parsedBatchItem) {
383+
func executeTestBatchWrites(ctx context.Context, gqlClient *githubv4.Client, items []resolvedBatchItem, results []batchItemResult) {
382384
executeBatchWrites(
383385
ctx,
384-
gqlClient,
385-
batchMutationUpdate,
386-
githubv4.ID("PVT_project"),
387-
githubv4.ID("PVTF_field"),
388-
githubv4.ProjectV2FieldValue{Text: githubv4.NewString("value")},
386+
batchWriteOperation{
387+
gqlClient: gqlClient,
388+
kind: batchMutationUpdate,
389+
projectID: githubv4.ID("PVT_project"),
390+
fieldID: githubv4.ID("PVTF_field"),
391+
value: githubv4.ProjectV2FieldValue{Text: githubv4.NewString("value")},
392+
},
389393
items,
390394
results,
391-
parsed,
392395
)
393396
}
394397

395-
func batchItemsOfSize(n int) ([]resolvedBatchItem, []parsedBatchItem, []batchItemResult) {
398+
func batchItemsOfSize(n int) ([]resolvedBatchItem, []batchItemResult) {
396399
items := make([]resolvedBatchItem, n)
397-
parsed := make([]parsedBatchItem, n)
398400
for i := range n {
399401
nodeID := fmt.Sprintf("PVTI_item%d", i)
400402
items[i] = resolvedBatchItem{
401403
index: i,
404+
ref: map[string]any{"node_id": nodeID},
402405
nodeID: nodeID,
403406
}
404-
parsed[i] = parsedBatchItem{
405-
index: i,
406-
ref: map[string]any{"node_id": nodeID},
407-
}
408407
}
409-
return items, parsed, make([]batchItemResult, n)
408+
return items, make([]batchItemResult, n)
410409
}
411410

412411
func Test_ExecuteAliasedMutation_EmptyInputsIsNoop(t *testing.T) {

0 commit comments

Comments
 (0)