feat(pi): Babysit foundations — shared PR/push extraction, five GitHub tools, sandbox lifetime#5962
feat(pi): Babysit foundations — shared PR/push extraction, five GitHub tools, sandbox lifetime#5962BillLeoutsakosvl346 wants to merge 23 commits into
Conversation
…ntation plan Carries the plan and review protocol into the repository so a cloud agent working from the remote can read them. Temporary: the plan is removed before this branch goes for review. Co-authored-by: Cursor <cursoragent@cursor.com>
…ls, sandbox lifetime Stage 1 of the Babysit plan (.agents/plans/pi-babysit-mode.plan.md), sections 0, 3, and 7 plus their tests. The Babysit mode itself lands in stage 2. Section 0 — shared extraction and push hardening: - Move PREPARE_SCRIPT, PUSH_SCRIPT, and the finalize path/size constants from cloud-backend.ts into cloud-shared.ts. - Move Review Code's PR snapshot helpers into pi/github-pr.ts, generalized off PiCloudReviewRunParams to a plain PullRequestCoordinates, and split the raw fetch from the "must be open" wrapper so a mode that has to report a closed PR gracefully can build on the raw form. - Harden the one token-bearing command: GIT_CONFIG_NOSYSTEM/GIT_CONFIG_GLOBAL on its env, git by absolute path, and an explicit HEAD:refs/heads/$BRANCH refspec. The clone script now emits a .git/config digest marker as its last line for every mode; only Babysit will verify it. Section 3 — five GitHub tools, registered but not wired into the block dropdown: github_list_review_threads, github_reply_review_thread, github_resolve_review_thread, github_status_check_rollup (GraphQL) and github_job_logs (REST). Plus a nullable repo_full_name on the PR reader's branch parse, declared on its own output rather than the shared BRANCH_REF_OUTPUT. Section 7 — CreateSandboxOptions.lifetimeMs threaded to E2B's timeoutMs, clamped below the one-hour Hobby ceiling and lowerable by PI_SANDBOX_LIFETIME_MS. Daytona is deliberately untouched. The per-command Pi timeout is capped at the lifetime.
…e token on redirect Two defects found in review, both verified against the live GitHub API. GraphQL's CheckRun has no `output` object — that is REST's shape. It exposes `title`, `summary`, and `text` as flat nullable fields, confirmed by introspecting the schema. The old selection made every github_status_check_rollup call fail with "Field 'output' doesn't exist on type 'CheckRun'", delivered as an HTTP 200 errors payload that no fixture-based test could catch. The corrected query was run against a real PR and returns 18 check runs plus a Vercel status context, with title and summary null on every Actions run exactly as the plan predicted. github_job_logs redirects to third-party blob storage, and Sim's tool fetch follows redirects itself rather than through the fetch spec, so it replayed the GitHub token to that host. Tools can now declare `stripAuthOnRedirect`, and the log reader does. Also from review: pin Review Code's "must be open" guard with a test now that it lives in its own function, drop the stream reader in github_job_logs since the executor already hands transformResponse a capped buffer, and correct three doc comments that overstated what they guaranteed.
Asserting the flag on the tool config alone would not catch a regression in formatRequestParams or in the executor's call into secureFetchWithPinnedIP, so pin what the fetch layer actually receives, in both the opted-in and the default case.
…hten isRequired Three review findings, each verified rather than taken on faith. PUSH_SCRIPT's comment claimed GIT_CONFIG_NOSYSTEM and GIT_CONFIG_GLOBAL close config-driven URL rewriting. They do not: reproduced locally on git 2.43, a repository-local url.*.insteadOf still rewrites the push URL and sends the token's userinfo to another host. That is the scope a root agent in the checkout can actually write, and it stays open until a mode verifies the config digest — which is Babysit, per the plan. The comment now says that instead of the opposite. PI_TIMEOUT_MS capped the Pi command at the whole sandbox lifetime, so the sandbox always died first and the stated benefit — a clean timeout instead of an opaque SDK error — could never happen. It now reserves the clone and finalize budgets it shares the sandbox with, leaving the host time to commit and push whatever the agent produced. isRequired is Boolean! on both CheckRun and StatusContext (confirmed by schema introspection), so the nullable parse modelled a value GitHub cannot send and left stage 2 a tri-state to handle. It is required now, and an absent value fails loudly rather than reading as "not required", which would let a failing required check stop blocking the green verdict. Also: a github-pr.test.ts pinning that the raw fetchPrSnapshot does not throw on a closed PR (the entire reason for the wrapper split, previously untested), a cloud-shared.test.ts for the timeout reserve and the digest line, the E2B lifetime ceiling documented next to E2B_PI_TEMPLATE_ID as section 7 asks, and the new registry tests no longer leaving their fake tools registered.
Create PR dispatches two commands at FINALIZE_TIMEOUT_MS, not one — the commit and the push — so reserving a single budget left the push unbudgeted and the worst case overshot the sandbox lifetime by exactly that amount. Losing the sandbox during the push is the most expensive moment to lose it: the work is committed and unpushed, which is the outcome the reserve exists to prevent. The comment no longer claims more than the arithmetic delivers. What is reserved is each command's timeout ceiling rather than its measured elapsed time, so this is a budget that adds up, not a guarantee. Two other comments described Babysit verifying the config digest in the present tense, when no mode verifies it yet. Also drops the digest-line test: it asserted a string constant contains its own substrings, while cloud-backend.test.ts already pins the property that matters — the marker being the clone's last line, after the remote rewrite.
Three comments still read as statements of current behavior: the Create PR push test's note beside the assertion that proves no verification happens, github-pr's module doc naming a second consumer that does not exist yet, and the timeout floor claiming a short-lifetime run was doomed regardless when the reserved ceilings are pessimistic enough that it may well finish.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThe PR adds optional Babysit Mode to Create PR and its supporting GitHub, sandbox-lifetime, orchestration, documentation, and test foundations.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported endpoint-confinement, provider-lifetime, and Babysit-budget issues are addressed by the current validation, normalization, reserve arithmetic, and provider-aware fallback paths.
|
| Filename | Overview |
|---|---|
| apps/sim/executor/handlers/pi/babysit-backend.ts | Implements provider-aware bounded Babysit orchestration, round gating, partial outcomes, and guarded GitHub-side finalization. |
| apps/sim/executor/handlers/pi/cloud-backend.ts | Composes Create PR with a sequential Babysit continuation while preserving the platform execution budget. |
| apps/sim/lib/execution/remote-sandbox/pi-lifetime.ts | Normalizes provider selection and clamps E2B lifetime to a reserve-compatible 31–59 minute range without applying an absolute lifetime to Daytona. |
| apps/sim/tools/github/job_logs.ts | Retrieves bounded job-log tails using encoded repository coordinates, validated job IDs, and authorization stripping on redirects. |
| apps/sim/blocks/blocks/pi.ts | Adds the Babysit configuration and conditional output surface to the Pi block. |
| apps/docs/content/docs/en/workflows/blocks/pi.mdx | Documents Babysit behavior, permissions, lifecycle, security boundaries, outputs, and expected partial-success states. |
Sequence Diagram
sequenceDiagram
participant User
participant CreatePR as Create PR Handler
participant Sandbox1 as Creation Sandbox
participant GitHub
participant Babysit as Babysit Orchestrator
participant Sandbox2 as Babysit Sandbox
User->>CreatePR: Run Create PR with Babysit enabled
CreatePR->>Sandbox1: Clone, edit, commit, and push
Sandbox1->>GitHub: Create ready-for-review PR
CreatePR->>Sandbox1: Destroy sandbox
CreatePR->>Babysit: Continue with pinned PR coordinates
Babysit->>GitHub: Post reviewer mentions
Babysit->>Sandbox2: Clone pinned PR head
loop Bounded fixing rounds
Babysit->>GitHub: Read review threads and check rollup
Babysit->>Sandbox2: Run Pi with trusted bounded context
Sandbox2->>Babysit: Decision file and one candidate commit
Babysit->>GitHub: Push exact head ref
Babysit->>GitHub: Reply, resolve, and request review
end
Babysit->>Sandbox2: Destroy sandbox
Babysit-->>CreatePR: Counters, booleans, and stop reason
CreatePR-->>User: PR URL and complete or partial result
Reviews (15): Last reviewed commit: "Wait for reviews before skipped-thread e..." | Re-trigger Greptile
… path Deriving PI_TIMEOUT_MS from the E2B lifetime applied it to every provider, so a Daytona Create PR run lost its ~90-minute agent turn to a ceiling Daytona does not have — it stops on inactivity instead. The reserve now only applies when the provider imposes an absolute lifetime. A configured PI_SANDBOX_LIFETIME_MS below the clone and finalize reserves left no positive remainder for the turn, so E2B could reap the sandbox before the push. Such a value is raised to a floor rather than rejected: a module-scope throw on a config typo would take down every path that imports this, not just Pi. github_job_logs returns its response body verbatim, so unlike its siblings that parse a typed shape, a coordinate carrying URL syntax turned a bearer-authenticated request into a general read. Path segments are now escaped and the job id checked. Also corrects the plan's rollup field path: GraphQL's CheckRun has no output object, and isRequired is Boolean!, so stage 2 needs neither the nested path nor an unknown-required branch. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6bd708c. Configure here.
|
@cursor review |
|
@cursor review |
|
@cursor review |
|
@cursor review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2dc74ef. Configure here.
|
@cursor review |
|
@cursor review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3c0c2fa. Configure here.










Summary
Stage 1 of the Babysit plan — sections 0, 3, and 7 only, plus the parts of section 8 that test them. The plan is split deliberately so the mechanical foundations land and get reviewed before the feature is built on top of them.
Section 0 — shared extraction and push hardening.
PREPARE_SCRIPT,PUSH_SCRIPT, and the finalize path/size constants move fromcloud-backend.tsintocloud-shared.ts. Review Code's PR-snapshot helpers move into a newpi/github-pr.ts, generalized offPiCloudReviewRunParamsto a plainPullRequestCoordinates, with the "must be open" guard split out offetchPrSnapshotinto afetchOpenPrSnapshotwrapper so a mode that has to report a closed PR gracefully can build on the raw form. Then the hardening on the one token-bearing command:GIT_CONFIG_NOSYSTEM/GIT_CONFIG_GLOBALon its env, git by absolute path, and an explicitHEAD:refs/heads/$BRANCHrefspec. The clone script emits a.git/configdigest marker as its last line, after the remote rewrite, for every mode that clones to push; verification is Babysit's and is not here.Section 3 — five GitHub tools plus one parser field.
github_list_review_threads,github_reply_review_thread,github_resolve_review_thread,github_status_check_rollup(GraphQL) andgithub_job_logs(REST), registered intools/registry.tsbut not wired into the GitHub block's operation dropdown. Plus a nullablerepo_full_nameon the PR reader's branch parse, declared on its own output rather than the sharedBRANCH_REF_OUTPUT_PROPERTIESthatlist_prsreuses.Section 7 — sandbox lifetime.
CreateSandboxOptions.lifetimeMsthreaded to E2B'stimeoutMs, clamped below the one-hour Hobby ceiling and lowerable byPI_SANDBOX_LIFETIME_MS. Daytona is untouched — itsautoStopIntervalis an inactivity interval with different semantics and a default that already outlasts these runs. This fixes a live bug: E2B's default reaps a sandbox after five minutes, so any Create PR or Review Code run past that dies today.Deferred to stage 2 (sections 1, 2, 4, 5, 6, 9): the
pi.tsblock surface, handler wiring,babysit-github.ts,babysit-round.ts,babysit-backend.ts, and the Babysit docs. Nothing is stubbed.Type of Change
Testing
Gates, all from the repo root on
785d37219:bun run testapps/sim1127 files / 14731 testsbun run type-checkbun run lint:check/format:checkcheck:boundaries,check:api-validation,check:realtime-prune,check:utils,check:client-boundary,skills:checkmship-tools:check8493ed4a2— see belowmship-tools:checkreads../copilot/copilot/contracts/tool-catalog-v1.jsonfrom a sibling repository that is not checked out in the agent environment. That catalog is the copilot agent-tool catalog and is not fed byapps/sim/tools/registry.ts, so registering integration tools cannot affect it;apps/sim/lib/copilot/generated/is untouched by this diff. Please confirm it passes in CI.Where to focus review:
apps/sim/tools/types.ts,tools/utils.ts,tools/index.ts—stripAuthOnRedirectis on the request path of every tool, not just the new ones. It is opt-in and defaults toundefined, and there is a test that a tool which does not set it is unaffected.PI_TIMEOUT_MS's reserve incloud-shared.ts— the one behavioral change to Create PR beyond the push command itself.Outstanding manual verification — not done, not claimed. Section 7 asks for a real Pi run lasting past five minutes against E2B to prove the lifetime fix end to end. The agent environment has no E2B credentials, so this is unit-tested only. Someone with credentials should run one Create PR before merge: this is the single change here with immediate production effect, and its failure mode —
Sandbox.createrejecting the requested lifetime — would take out both cloud Pi modes, which is worse than the bug it fixes. The risk is low (the installed SDK's typings name 3,600,000 ms as the Hobby ceiling and we request 3,540,000), andPI_SANDBOX_LIFETIME_MSlowers it without a redeploy if needed.Plan discrepancies found
Two, both verified against the live GitHub API rather than argued. I have not edited the plan — stage 2 depends on it, so these are reported here for a human to fold in.
Section 3's rollup query is wrong about
CheckRun.output, and it would have failed 100% of the time. The plan specifiesoutput { title summary }in theCheckRunfragment, and section 5 reasons from "output.title,summary, ortext". That is REST's shape. GraphQL'sCheckRunhas nooutputfield; introspection returns flat nullabletitle,summary, andtext. GitHub answers the invalid selection with HTTP 200 anderrors: [{ message: "Field 'output' doesn't exist on type 'CheckRun'" }], so every call would have raised — and no fixture-based test could have caught it, because the fixture would have been built from the same wrong assumption. Implemented with the flat fields; the corrected query was run against a live PR and returns 18 check runs plus a Vercel status context. The plan's underlying claims all hold: Actions leavestitleandsummarynull on every run, and a check run'sdatabaseIdis the job id in its owndetailsUrl. Sections 3, 4, and 5 need the field path corrected.Section 4's
isRequired"unknown" state is unreachable. The plan says a failing check blocks the green verdict "whenisRequiredis true or unknown".isRequiredisBoolean!on bothCheckRunandStatusContext(introspected), and a field error on a non-null field nulls the whole node and emits anerrorsarray, which the shared reader rejects before parsing. Parsed as required, so an absent value fails loudly rather than defaulting — defaulting it either way would let a failing required check stop blocking the verdict. Stage 2 can drop the tri-state branch.Review findings pushed back on
Four cycles of independent review across two model families produced these; each is recorded with the reasoning rather than silently dropped.
github_job_logscan tail a log over 10 MB. Declined. The executor caps a response at 10 MB and stops with a clear size-limit error, which is the right outcome: a head-truncated log would read as a passing job. Bypassing the SSRF-protected buffered read for one tool is a change to shared security machinery for a rare case. I did take the half that was right — the streaming reader I had written bounded memory that the executor had already bounded, so it is gone in favour ofresponse.text()plus a slice. Both reviewers accepted; one noted stage 2 should treat a size failure on a diagnostic read as "diagnostics unavailable for this check" rather than folding it into the fail-closed check-state rule.secureFetchWithPinnedIPstripAuthorizationon any cross-origin redirect by default. Declined here. The default is wrong in the abstract and the siblingfollowRedirectsGuardedin the same file already does it, but flipping it changes behavior for every tool in the product from inside a Pi-foundations commit, and the sweep possible from here established only that the GitHub family is unaffected. Recorded as a deliberate decision, worth its own change with its own sweep. Reviewer accepted.PI_SANDBOX_LIFETIME_MSwith a setup error. Declined.PI_TIMEOUT_MSis a module constant, so that is a throw at import time — one mistyped env var would take down every path that transitively importscloud-shared.ts, including ones that never touch a sandbox. The floor plus an accurate comment is the proportionate answer. Reviewer accepted.reviews(last: 1)tolast: 3so a human review landing after a bot's cannot hide it. Declined. Plan-specified, only exercisable in stage 2, and the failure mode is a wasted round ending in an honestawaiting_reviewrather than a false clean. Filed for whoever writesreviewLandedSince— and note the correction a reviewer made to my reasoning: the issue-comment path does not backstop this, since the two halves are complementary by design.//comment as violating the TSDoc-only convention. Declined. The cited comments state constraints the code cannot show, and the surrounding Pi and tools files are written the same way. Reviewer retracted the finding.Defects found in review and fixed
Beyond the two plan discrepancies: the
github_job_logsendpoint 302s to third-party blob storage, and the tool fetch path follows redirects itself rather than through WHATWG fetch, so it was replaying the GitHub token to that host — fixed with the newstripAuthOnRedirect, and the leak was confirmed live before and after.PUSH_SCRIPT's comment claimedGIT_CONFIG_NOSYSTEM/GIT_CONFIG_GLOBALclose config-driven URL rewriting; reproduced locally on git 2.43 that a repository-localurl.*.insteadOfstill redirects the token, so the comment now says what the pair actually buys and names what stays open. AndPI_TIMEOUT_MSoriginally capped at the bare sandbox lifetime, so the sandbox always won the race and the agent's finished work would be lost before the push; it now reserves the clone and both finalize budgets.Checklist