Skip to content

Commit 8c7d9d2

Browse files
committed
fix(pi): correct Babysit check, budget, and push-guard accuracy
Correctness: - The `.github/` push refusal compared raw `git diff --name-only` output, which git C-quotes for non-ASCII paths. `.github/workflows/évil.yml` arrived as `".github/workflows/\303\251vil.yml"` — leading quote included — so neither `.github` nor `.github/` matched and the file pushed. Both name-listing diffs now pin `core.quotePath=false`; Create PR's does too so `changedFiles` reports real names. - `CANCELLED` and `STALE` were counted as non-failing conclusions, so a cancelled required check produced `checksGreen: true` and `stopReason: 'clean'` on a PR branch protection still blocks. Both now fall through to failing, matching every other unknown conclusion. - The per-round check bound counted optional checks, so a repo with a wide optional matrix ended the run at `bounds_exceeded` before a single review thread was addressed. Only required-check overflow is fatal now; the rest trims, required checks first, and reports what was left out. This also makes the prompt's existing slice reachable. - A re-review request that posted nothing still re-armed `requestedAt` with `landed: false`, leaving the loop waiting on a review nobody had asked for — burning the remaining lifetime on a billed idle sandbox before reporting `awaiting_review` on a clean PR. The previous request now stands. - Prompt bounds threw where every other bound in the file trims, ending a busy PR's run on round one after the PR and its review comments were already posted. They now drop trailing entries and note the omission. - `babysitMode` used a strict boolean compare; a `switch` input arriving as the string 'true' silently opened a draft PR and skipped Babysit while the editor showed it enabled. Matches `wait-handler`'s coercion now. - The fork check compared head against the block's typed owner/repo, so a renamed repository — which GitHub serves through a 301 while reporting the canonical name — was reported as a fork. Compares head against base now. - Each round's diff is capped like Create PR's. The cumulative guard measures the net change, so a round that reverts an earlier addition passed it while contributing a full-size diff. - Reviewer mentions must start with `@`. Each entry becomes its own issue comment re-posted every round, so a comma inside one left prose on the PR. Efficiency: - Thread and check reads per poll now run together; neither consumes the other's result and both are paginating loops. - `babysitReviewLandedSince` takes the `latestReview` the caller already fetched instead of re-listing the PR for it. - Actions log reads fan out in small batches rather than one at a time. Cleanup: - `BabysitFinalizeError` was a twin of `BabysitGitHubError`; folded together. - Replaced the hand-inlined copies of `threadsAreClean`. - Dropped `MEMORY_MODES`, a duplicate of `AUTHORING_MODES`, and the `parsePiMode` tombstone for a mode that never shipped. Adds regression tests for the quoted `.github` path, cancelled/stale required checks, and the renamed-repository snapshot.
1 parent 810844c commit 8c7d9d2

11 files changed

Lines changed: 415 additions & 200 deletions

apps/sim/blocks/blocks/pi.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,6 @@ const AUTHORING_MODES: { field: 'mode'; value: Array<'cloud' | 'local'> } = {
7575
field: 'mode',
7676
value: ['cloud', 'local'],
7777
}
78-
const MEMORY_MODES: { field: 'mode'; value: Array<'cloud' | 'local'> } = {
79-
field: 'mode',
80-
value: ['cloud', 'local'],
81-
}
8278
const MEMORY_TYPES = ['conversation', 'sliding_window', 'sliding_window_tokens']
8379

8480
const SEARCH_PROVIDER_OPTIONS = [
@@ -467,7 +463,7 @@ export const PiBlock: BlockConfig<PiResponse> = {
467463
{ label: 'Sliding window (tokens)', id: 'sliding_window_tokens' },
468464
],
469465
mode: 'advanced',
470-
condition: MEMORY_MODES,
466+
condition: AUTHORING_MODES,
471467
},
472468
{
473469
id: 'conversationId',

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,32 @@ describe('runBabysitPiWithOptions', () => {
626626
expect(runCalls.some(({ command }) => command.includes('CURRENT_DIGEST='))).toBe(false)
627627
})
628628

629+
// The prepare script pins `core.quotePath=false`, so a non-ASCII path arrives verbatim
630+
// rather than as git's default `".github/workflows/\303\251vil.yml"` rendering — which
631+
// begins with a quote character and so matched neither `.github` nor `.github/`.
632+
it('refuses a .github path that git would otherwise C-quote', async () => {
633+
mockFetchSnapshot.mockResolvedValue(snapshot)
634+
mockFetchThreads.mockResolvedValue({
635+
actionable: [trustedThread],
636+
skipped: [],
637+
totalUnresolved: 1,
638+
latestReview: null,
639+
})
640+
mockFetchChecks.mockResolvedValue(greenChecks)
641+
const unicodePath = '.github/workflows/évil.yml'
642+
const { runner, runCalls } = makeRunner({
643+
prepareStdout: `__CUMULATIVE_CHANGED__=${unicodePath}\n__CUMULATIVE_DIFF_BYTES__=20\n__CHANGED__=${unicodePath}\n__NEW_SHA__=${NEW_SHA}\n__NEEDS_PUSH__=1\n`,
644+
})
645+
mockWithPiSandbox.mockImplementation(async (callback) => callback(runner))
646+
647+
const result = await runBabysitPiWithOptions(params(), { onEvent: vi.fn() })
648+
649+
expect(result).toMatchObject({ stopReason: 'refused_content', commitsPushed: 0 })
650+
expect(runCalls.some(({ command }) => command.includes('CURRENT_DIGEST='))).toBe(false)
651+
const prepare = runCalls.find(({ command }) => command.includes('__CUMULATIVE_CHANGED__'))
652+
expect(prepare?.command).toContain('core.quotePath=false')
653+
})
654+
629655
it('reports a hardened push rejection without losing partial counters', async () => {
630656
mockFetchSnapshot.mockResolvedValue(snapshot)
631657
mockFetchThreads.mockResolvedValue({

0 commit comments

Comments
 (0)