Skip to content

Commit dbf510f

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(pi): preserve babysit partial state
1 parent 49c1349 commit dbf510f

4 files changed

Lines changed: 139 additions & 14 deletions

File tree

apps/sim/executor/handlers/pi/babysit-backend.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ describe('runBabysitPiWithOptions', () => {
236236
mockReplyAndResolve.mockResolvedValue({
237237
repliesPosted: 1,
238238
threadsResolved: 1,
239+
resolvedThreadIds: ['thread-1'],
239240
replyFailures: [],
240241
resolveFailures: [],
241242
headMoved: false,
@@ -646,6 +647,43 @@ describe('runBabysitPiWithOptions', () => {
646647
expect(mockReplyAndResolve.mock.calls[0][4]).toBe(OLD_SHA)
647648
})
648649

650+
it('preserves known clean flags when the pin moves after successful writes', async () => {
651+
const noChecksGreen = {
652+
...greenChecks,
653+
checks: [],
654+
contextRequirements: new Map<string, boolean>(),
655+
}
656+
mockFetchSnapshot
657+
.mockResolvedValueOnce(snapshot)
658+
.mockResolvedValueOnce(snapshot)
659+
.mockResolvedValueOnce({ ...snapshot, headSha: NEW_SHA })
660+
.mockResolvedValueOnce({ ...snapshot, headSha: SECOND_SHA })
661+
mockFetchThreads.mockResolvedValue({
662+
actionable: [trustedThread],
663+
skipped: [],
664+
totalUnresolved: 1,
665+
latestReview: null,
666+
})
667+
mockFetchChecks.mockResolvedValue(noChecksGreen)
668+
const { runner } = makeRunner({})
669+
mockWithPiSandbox.mockImplementation(async (callback) => callback(runner))
670+
671+
const result = await runBabysitPiWithOptions(
672+
params({ reviewMentions: ['@review-bot'] }),
673+
{ onEvent: vi.fn() },
674+
{ convergenceWaitMs: 0, roundWaitMs: 0 }
675+
)
676+
677+
expect(result).toMatchObject({
678+
stopReason: 'head_moved',
679+
rounds: 1,
680+
commitsPushed: 1,
681+
threadsResolved: 1,
682+
threadsClean: true,
683+
checksGreen: true,
684+
})
685+
})
686+
649687
it('reports a confirmed push when the convergence read fails transiently', async () => {
650688
mockFetchSnapshot
651689
.mockResolvedValueOnce(snapshot)

apps/sim/executor/handlers/pi/babysit-backend.ts

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,7 @@ export async function runBabysitPiWithOptions(
598598

599599
let latestThreads: BabysitThreadsState | undefined
600600
let latestChecks: BabysitCheckState | undefined
601+
let lastKnownChecksGreen = false
601602
let githubWriteOccurred = false
602603
try {
603604
if (signal.aborted) throw new Error('Pi run aborted')
@@ -617,6 +618,7 @@ export async function runBabysitPiWithOptions(
617618
}
618619
latestThreads = await fetchBabysitThreads(params, signal)
619620
latestChecks = await fetchBabysitCheckState(params, pinnedHeadSha, undefined, signal)
621+
lastKnownChecksGreen = latestChecks.checksGreen
620622
const initialRequirements = latestChecks.contextRequirements
621623
if (latestChecks.startupFailure) {
622624
const threadsClean =
@@ -726,6 +728,7 @@ export async function runBabysitPiWithOptions(
726728
initialRequirements,
727729
signal
728730
)
731+
lastKnownChecksGreen = latestChecks.checksGreen
729732
if (reviewRequest && !reviewRequest.landed) {
730733
reviewRequest.landed = await babysitReviewLandedSince(
731734
params,
@@ -853,6 +856,7 @@ export async function runBabysitPiWithOptions(
853856
githubWriteOccurred = true
854857
progress.changedFiles = finalized.changedFiles
855858
progress.diff = finalized.diff
859+
lastKnownChecksGreen = ![...initialRequirements.values()].some((required) => required)
856860
let convergence: Awaited<ReturnType<typeof waitForHeadConvergence>>
857861
try {
858862
convergence = await waitForHeadConvergence(
@@ -938,6 +942,16 @@ export async function runBabysitPiWithOptions(
938942
laggingHeadSha
939943
)
940944
progress.threadsResolved += writeResult.threadsResolved
945+
const resolvedThreadIds = new Set(writeResult.resolvedThreadIds ?? [])
946+
if (resolvedThreadIds.size > 0) {
947+
latestThreads = {
948+
...latestThreads,
949+
actionable: latestThreads.actionable.filter(
950+
(thread) => !resolvedThreadIds.has(thread.id)
951+
),
952+
totalUnresolved: Math.max(0, latestThreads.totalUnresolved - resolvedThreadIds.size),
953+
}
954+
}
941955
githubWriteOccurred ||= writeResult.repliesPosted > 0 || writeResult.threadsResolved > 0
942956
if (writeResult.replyFailures.length) {
943957
progress.notes.push(`${writeResult.replyFailures.length} thread replies failed.`)
@@ -951,7 +965,15 @@ export async function runBabysitPiWithOptions(
951965
)
952966
}
953967
if (writeResult.stopReason) {
954-
return resultFor(totals, writeResult.stopReason, progress, threadsClean, false)
968+
const knownThreadsClean =
969+
latestThreads.actionable.length === 0 && latestThreads.skipped.length === 0
970+
return resultFor(
971+
totals,
972+
writeResult.stopReason,
973+
progress,
974+
knownThreadsClean,
975+
lastKnownChecksGreen
976+
)
955977
}
956978
if (writeResult.phaseError) {
957979
progress.notes.push(
@@ -964,15 +986,27 @@ export async function runBabysitPiWithOptions(
964986
totals,
965987
finalized.commitPushed ? 'pushed_awaiting_confirmation' : 'agent_failure',
966988
progress,
967-
threadsClean,
968-
false
989+
latestThreads.actionable.length === 0 && latestThreads.skipped.length === 0,
990+
lastKnownChecksGreen
969991
)
970992
}
971993
if (writeResult.headMoved) {
972-
return resultFor(totals, 'head_moved', progress, threadsClean, false)
994+
return resultFor(
995+
totals,
996+
'head_moved',
997+
progress,
998+
latestThreads.actionable.length === 0 && latestThreads.skipped.length === 0,
999+
lastKnownChecksGreen
1000+
)
9731001
}
9741002
if (writeResult.awaitingConfirmation) {
975-
return resultFor(totals, 'pushed_awaiting_confirmation', progress, threadsClean, false)
1003+
return resultFor(
1004+
totals,
1005+
'pushed_awaiting_confirmation',
1006+
progress,
1007+
latestThreads.actionable.length === 0 && latestThreads.skipped.length === 0,
1008+
lastKnownChecksGreen
1009+
)
9761010
}
9771011

9781012
if (finalized.commitPushed && params.reviewMentions.length > 0) {
@@ -1016,6 +1050,7 @@ export async function runBabysitPiWithOptions(
10161050
initialRequirements,
10171051
signal
10181052
)
1053+
lastKnownChecksGreen = latestChecks.checksGreen
10191054
if (reviewRequest) {
10201055
reviewRequest.landed = await babysitReviewLandedSince(
10211056
params,
@@ -1089,7 +1124,7 @@ export async function runBabysitPiWithOptions(
10891124
githubError.reason as BabysitStopReason,
10901125
progress,
10911126
threadsClean,
1092-
latestChecks?.checksGreen ?? false
1127+
lastKnownChecksGreen
10931128
)
10941129
}
10951130
if (githubWriteOccurred) {
@@ -1103,8 +1138,10 @@ export async function runBabysitPiWithOptions(
11031138
totals,
11041139
progress.commitsPushed > 0 ? 'pushed_awaiting_confirmation' : 'agent_failure',
11051140
progress,
1106-
false,
1107-
false
1141+
!!latestThreads &&
1142+
latestThreads.actionable.length === 0 &&
1143+
latestThreads.skipped.length === 0,
1144+
lastKnownChecksGreen
11081145
)
11091146
}
11101147
throw createScrubbedPiError(error, secrets, 'Babysit failed')

apps/sim/executor/handlers/pi/babysit-github.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,47 @@ describe('Babysit GitHub orchestration', () => {
340340
)
341341
})
342342

343+
it('reports awaiting confirmation when a third SHA appears after a lagging push', async () => {
344+
const thirdSha = 'd'.repeat(40)
345+
mockExecuteTool.mockImplementation(async (toolId: string) => {
346+
if (toolId === 'github_reply_review_thread') {
347+
return { success: true, output: { id: 'reply' } }
348+
}
349+
if (toolId === 'github_pr_v2') {
350+
return {
351+
success: true,
352+
output: snapshot({
353+
head: { sha: thirdSha, ref: 'feature', repo_full_name: 'octo/demo' },
354+
}),
355+
}
356+
}
357+
throw new Error(`Unexpected tool ${toolId}`)
358+
})
359+
360+
const result = await replyAndResolveBabysitThreads(
361+
params,
362+
{ headSha: NEXT_SHA, headRef: 'feature', baseRef: 'main' },
363+
[
364+
{
365+
threadId: 'one',
366+
classification: 'fixed',
367+
reply: 'Fixed.',
368+
resolvable: true,
369+
},
370+
],
371+
undefined,
372+
HEAD_SHA
373+
)
374+
375+
expect(result).toMatchObject({
376+
repliesPosted: 1,
377+
threadsResolved: 0,
378+
headMoved: false,
379+
awaitingConfirmation: true,
380+
})
381+
expect(result.stopReason).toBeUndefined()
382+
})
383+
343384
it('retains successful reply counts when between-phase revalidation fails', async () => {
344385
mockExecuteTool.mockImplementation(async (toolId: string) => {
345386
if (toolId === 'github_reply_review_thread') {

apps/sim/executor/handlers/pi/babysit-github.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ export interface BabysitCheckState {
103103
export interface BabysitReplyResolveResult {
104104
repliesPosted: number
105105
threadsResolved: number
106+
resolvedThreadIds: string[]
106107
replyFailures: string[]
107108
resolveFailures: string[]
108109
headMoved: boolean
@@ -575,6 +576,7 @@ export async function replyAndResolveBabysitThreads(
575576
return {
576577
repliesPosted: replySuccesses.length,
577578
threadsResolved: 0,
579+
resolvedThreadIds: [],
578580
replyFailures,
579581
resolveFailures: [],
580582
headMoved: false,
@@ -584,15 +586,21 @@ export async function replyAndResolveBabysitThreads(
584586
assertBabysitPinned(pin, current)
585587
} catch (error) {
586588
if (signal?.aborted) throw error
589+
const laggingHeadMoved =
590+
error instanceof BabysitGitHubError && error.reason === 'head_moved' && !!laggingHeadSha
587591
return {
588592
repliesPosted: replySuccesses.length,
589593
threadsResolved: 0,
594+
resolvedThreadIds: [],
590595
replyFailures,
591596
resolveFailures: [],
592-
headMoved: error instanceof BabysitGitHubError && error.reason === 'head_moved',
593-
awaitingConfirmation: false,
597+
headMoved:
598+
error instanceof BabysitGitHubError && error.reason === 'head_moved' && !laggingHeadMoved,
599+
awaitingConfirmation: laggingHeadMoved,
594600
...(error instanceof BabysitGitHubError
595-
? { stopReason: error.reason }
601+
? laggingHeadMoved
602+
? {}
603+
: { stopReason: error.reason }
596604
: {
597605
phaseError: scrubPiSecrets(
598606
getErrorMessage(error, 'Failed to revalidate the pull request after replying'),
@@ -602,7 +610,7 @@ export async function replyAndResolveBabysitThreads(
602610
}
603611
}
604612

605-
let threadsResolved = 0
613+
const resolvedThreadIds: string[] = []
606614
const resolveFailures: string[] = []
607615
for (const decision of replySuccesses.filter((item) => item.resolvable)) {
608616
try {
@@ -611,7 +619,7 @@ export async function replyAndResolveBabysitThreads(
611619
{ threadId: decision.threadId, apiKey: params.githubToken },
612620
{ signal }
613621
)
614-
if (result.success) threadsResolved += 1
622+
if (result.success) resolvedThreadIds.push(decision.threadId)
615623
else resolveFailures.push(decision.threadId)
616624
} catch (error) {
617625
if (signal?.aborted) throw error
@@ -620,7 +628,8 @@ export async function replyAndResolveBabysitThreads(
620628
}
621629
return {
622630
repliesPosted: replySuccesses.length,
623-
threadsResolved,
631+
threadsResolved: resolvedThreadIds.length,
632+
resolvedThreadIds,
624633
replyFailures,
625634
resolveFailures,
626635
headMoved: false,

0 commit comments

Comments
 (0)