Skip to content

feat(pi): Babysit foundations — shared PR/push extraction, five GitHub tools, sandbox lifetime#5962

Open
BillLeoutsakosvl346 wants to merge 23 commits into
feature/pi-searchfrom
feature/pi-babysit
Open

feat(pi): Babysit foundations — shared PR/push extraction, five GitHub tools, sandbox lifetime#5962
BillLeoutsakosvl346 wants to merge 23 commits into
feature/pi-searchfrom
feature/pi-babysit

Conversation

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor

Summary

Stage 1 of the Babysit plansections 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 from cloud-backend.ts into cloud-shared.ts. Review Code's PR-snapshot helpers move into a new pi/github-pr.ts, generalized off PiCloudReviewRunParams to a plain PullRequestCoordinates, with the "must be open" guard split out of fetchPrSnapshot into a fetchOpenPrSnapshot wrapper 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_GLOBAL on its env, git by absolute path, and an explicit HEAD:refs/heads/$BRANCH refspec. The clone script emits a .git/config digest 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) and github_job_logs (REST), registered in tools/registry.ts but not wired into the GitHub block's operation dropdown. 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_PROPERTIES that list_prs reuses.

Section 7 — sandbox lifetime. CreateSandboxOptions.lifetimeMs threaded to E2B's timeoutMs, clamped below the one-hour Hobby ceiling and lowerable by PI_SANDBOX_LIFETIME_MS. Daytona is untouched — its autoStopInterval is 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.ts block surface, handler wiring, babysit-github.ts, babysit-round.ts, babysit-backend.ts, and the Babysit docs. Nothing is stubbed.

Type of Change

  • Bug fix — E2B reaps a Pi sandbox after five minutes today
  • New feature — the five GitHub tools
  • Breaking change
  • Documentation — the E2B lifetime ceiling, per section 7
  • Other: refactor (shared extraction) and security hardening

Testing

Gates, all from the repo root on 785d37219:

Gate Result
bun run test 12/12 workspaces; apps/sim 1127 files / 14731 tests
bun run type-check 19/19
bun run lint:check / format:check pass
check:boundaries, check:api-validation, check:realtime-prune, check:utils, check:client-boundary, skills:check pass
mship-tools:check fails identically on the parent 8493ed4a2 — see below

mship-tools:check reads ../copilot/copilot/contracts/tool-catalog-v1.json from a sibling repository that is not checked out in the agent environment. That catalog is the copilot agent-tool catalog and is not fed by apps/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.tsstripAuthOnRedirect is on the request path of every tool, not just the new ones. It is opt-in and defaults to undefined, and there is a test that a tool which does not set it is unaffected.
  • The four GraphQL queries. Every field was checked against the live schema by introspection, and the rollup and review-thread queries were run against a real open PR on this repository.
  • PI_TIMEOUT_MS's reserve in cloud-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.create rejecting 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), and PI_SANDBOX_LIFETIME_MS lowers 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.

  1. Section 3's rollup query is wrong about CheckRun.output, and it would have failed 100% of the time. The plan specifies output { title summary } in the CheckRun fragment, and section 5 reasons from "output.title, summary, or text". That is REST's shape. GraphQL's CheckRun has no output field; introspection returns flat nullable title, summary, and text. GitHub answers the invalid selection with HTTP 200 and errors: [{ 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 leaves title and summary null on every run, and a check run's databaseId is the job id in its own detailsUrl. Sections 3, 4, and 5 need the field path corrected.

  2. Section 4's isRequired "unknown" state is unreachable. The plan says a failing check blocks the green verdict "when isRequired is true or unknown". isRequired is Boolean! on both CheckRun and StatusContext (introspected), and a field error on a non-null field nulls the whole node and emits an errors array, 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.

  • Add a streaming-response path to the tool executor so github_job_logs can 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 of response.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.
  • Make secureFetchWithPinnedIP strip Authorization on any cross-origin redirect by default. Declined here. The default is wrong in the abstract and the sibling followRedirectsGuarded in 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.
  • Reject a too-low configured PI_SANDBOX_LIFETIME_MS with a setup error. Declined. PI_TIMEOUT_MS is a module constant, so that is a throw at import time — one mistyped env var would take down every path that transitively imports cloud-shared.ts, including ones that never touch a sandbox. The floor plus an accurate comment is the proportionate answer. Reviewer accepted.
  • Widen reviews(last: 1) to last: 3 so 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 honest awaiting_review rather than a false clean. Filed for whoever writes reviewLandedSince — 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.
  • Split section 7 into its own commit. Declined. The plan asks for it and the reasoning is sound, but the commit was already pushed to a shared branch a second agent will build on, and splitting means rewriting published history. Subsequent work went in as separate logical commits instead. Reviewer accepted.
  • Treat every // 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_logs endpoint 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 new stripAuthOnRedirect, and the leak was confirmed live before and after. PUSH_SCRIPT's comment claimed GIT_CONFIG_NOSYSTEM/GIT_CONFIG_GLOBAL close config-driven URL rewriting; reproduced locally on git 2.43 that a repository-local url.*.insteadOf still redirects the token, so the comment now says what the pair actually buys and names what stays open. And PI_TIMEOUT_MS originally 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

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)
Open in Web Open in Cursor 

Bill Leoutsakos and others added 7 commits July 25, 2026 07:47
…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.
@vercel

vercel Bot commented Jul 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 26, 2026 7:18pm

Request Review

@BillLeoutsakosvl346
BillLeoutsakosvl346 marked this pull request as ready for review July 25, 2026 17:13
@greptile-apps

greptile-apps Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds optional Babysit Mode to Create PR and its supporting GitHub, sandbox-lifetime, orchestration, documentation, and test foundations.

  • Adds bounded review/check polling, trusted-feedback handling, one-commit fixing rounds, host-side replies and resolution, and partial-success reporting.
  • Adds five GitHub operations with guarded redirects and endpoint-constrained job-log retrieval.
  • Makes E2B sandbox lifetime configurable within safe bounds while preserving Daytona’s provider-specific lifecycle.
  • Refactors shared PR, clone, push, and finalization behavior and documents the resulting workflow.

Confidence Score: 5/5

The 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.

Important Files Changed

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
Loading

Reviews (15): Last reviewed commit: "Wait for reviews before skipped-thread e..." | Re-trigger Greptile

Comment thread apps/sim/tools/github/job_logs.ts
Comment thread apps/sim/lib/execution/remote-sandbox/pi-lifetime.ts Outdated
Comment thread apps/sim/executor/handlers/pi/cloud-shared.ts Outdated
… 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>
@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@greptile

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

Comment thread apps/sim/executor/handlers/pi/babysit-backend.ts Outdated
@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@cursor review

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@greptile

Comment thread apps/sim/executor/handlers/pi/babysit-backend.ts Outdated
Comment thread apps/sim/executor/handlers/pi/babysit-backend.ts
Comment thread apps/sim/executor/handlers/pi/babysit-backend.ts Outdated
@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@cursor review

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@greptile

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@cursor review

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@greptile

Comment thread apps/sim/executor/handlers/pi/babysit-backend.ts Outdated
@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@cursor review

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@greptile

Comment thread apps/sim/lib/execution/remote-sandbox/pi-lifetime.ts Outdated
@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@cursor review

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@greptile

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@greptile

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@cursor review

Comment thread apps/sim/executor/handlers/pi/babysit-backend.ts Outdated
Comment thread apps/sim/executor/handlers/pi/babysit-backend.ts
@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@greptile

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@cursor review

Comment thread apps/sim/executor/handlers/pi/babysit-backend.ts
@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@greptile

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

@BillLeoutsakosvl346

Copy link
Copy Markdown
Contributor Author

Demo would be >30 mins to go back and forth 3 times, so I am uploading images instead, it worked:

Screenshot 2026-07-26 at 12 04 06 PM Screenshot 2026-07-26 at 12 04 25 PM Screenshot 2026-07-26 at 12 08 35 PM Screenshot 2026-07-26 at 12 04 37 PM Screenshot 2026-07-26 at 12 04 47 PM Screenshot 2026-07-26 at 12 04 58 PM Screenshot 2026-07-26 at 12 05 13 PM Screenshot 2026-07-26 at 12 05 20 PM Screenshot 2026-07-26 at 12 05 27 PM Screenshot 2026-07-26 at 12 05 42 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants