fix(pi): scope the sandbox lifetime cap to E2B and harden the job-log 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>

Author: 2 people

.agents/plans/pi-babysit-mode.plan.md

@@ -2,10 +2,14 @@
  * @vitest-environment node
  */
 import { describe, expect, it } from 'vitest'
-import { PI_SANDBOX_MAX_LIFETIME_MS } from '@/lib/execution/remote-sandbox/pi-lifetime'
+import {
+  PI_SANDBOX_MAX_LIFETIME_MS,
+  PI_SANDBOX_MIN_LIFETIME_MS,
+} from '@/lib/execution/remote-sandbox/pi-lifetime'
 import {
   CLONE_TIMEOUT_MS,
   FINALIZE_TIMEOUT_MS,
+  MIN_PI_TIMEOUT_MS,
   PI_TIMEOUT_MS,
 } from '@/executor/handlers/pi/cloud-shared'
 
@@ -21,4 +25,13 @@ describe('PI_TIMEOUT_MS', () => {
     )
     expect(PI_TIMEOUT_MS).toBeGreaterThan(0)
   })
+
+  it('keeps the lifetime floor above the reserves it exists to protect', () => {
+    // The floor lives next to the lifetime and the reserves live here, so
+    // without this they can drift until a permitted lifetime leaves the agent
+    // turn with nothing but its own minimum.
+    expect(PI_SANDBOX_MIN_LIFETIME_MS).toBeGreaterThanOrEqual(
+      CLONE_TIMEOUT_MS + 2 * FINALIZE_TIMEOUT_MS + MIN_PI_TIMEOUT_MS
+    )
+  })
 })

apps/sim/executor/handlers/pi/cloud-shared.test.ts

@@ -25,7 +25,7 @@ export const PUSH_ERROR_MAX = 1000
  * Such a run can still finish, since those ceilings are pessimistic, so the floor
  * leaves a short turn rather than refusing one.
  */
-const MIN_PI_TIMEOUT_MS = 60 * 1000
+export const MIN_PI_TIMEOUT_MS = 60 * 1000
 
 /**
  * How long one Pi CLI invocation may run. The platform's max execution timeout
@@ -41,14 +41,23 @@ const MIN_PI_TIMEOUT_MS = 60 * 1000
  * What is reserved is each command's timeout ceiling, not its measured elapsed
  * time — a clone takes seconds in practice — so this is a budget that adds up,
  * not a guarantee that the sandbox outlives the run.
+ *
+ * The reserve only applies when the provider imposes an absolute lifetime, which
+ * is E2B alone. Daytona stops on inactivity, so subtracting E2B's ceiling there
+ * would cut the agent turn to fit a limit Daytona does not have.
  */
-export const PI_TIMEOUT_MS = Math.min(
-  getMaxExecutionTimeout(),
-  Math.max(
-    resolvePiSandboxLifetimeMs() - CLONE_TIMEOUT_MS - 2 * FINALIZE_TIMEOUT_MS,
-    MIN_PI_TIMEOUT_MS
-  )
-)
+const piSandboxLifetimeMs = resolvePiSandboxLifetimeMs()
+
+export const PI_TIMEOUT_MS =
+  piSandboxLifetimeMs === undefined
+    ? getMaxExecutionTimeout()
+    : Math.min(
+        getMaxExecutionTimeout(),
+        Math.max(
+          piSandboxLifetimeMs - CLONE_TIMEOUT_MS - 2 * FINALIZE_TIMEOUT_MS,
+          MIN_PI_TIMEOUT_MS
+        )
+      )
 
 /**
  * Marker carrying a digest of the cloned repository's git config. A clone script

apps/sim/executor/handlers/pi/cloud-shared.ts

@@ -76,7 +76,10 @@ import {
   SIM_RESULT_PREFIX,
   withPiSandbox,
 } from '@/lib/execution/remote-sandbox'
-import { PI_SANDBOX_MAX_LIFETIME_MS } from '@/lib/execution/remote-sandbox/pi-lifetime'
+import {
+  PI_SANDBOX_MAX_LIFETIME_MS,
+  PI_SANDBOX_MIN_LIFETIME_MS,
+} from '@/lib/execution/remote-sandbox/pi-lifetime'
 
 type Provider = 'e2b' | 'daytona'
 const PROVIDERS: Provider[] = ['e2b', 'daytona']
@@ -483,13 +486,24 @@ describe('Pi sandbox lifetime', () => {
     expect(mockE2BCreate.mock.calls[0][1].timeoutMs).toBe(PI_SANDBOX_MAX_LIFETIME_MS)
   })
 
-  it('honours a configured lifetime below the ceiling', async () => {
+  it('honours a configured lifetime between the floor and the ceiling', async () => {
+    useProvider('e2b')
+    mockEnv.PI_SANDBOX_LIFETIME_MS = '2700000'
+
+    await withPiSandbox(async () => undefined)
+
+    expect(mockE2BCreate.mock.calls[0][1].timeoutMs).toBe(2_700_000)
+  })
+
+  it('raises a configured lifetime too short to clone, run, and push in', async () => {
     useProvider('e2b')
+    // Ten minutes is the clone reserve on its own, so the turn and the push would
+    // race a sandbox E2B may already have reaped.
     mockEnv.PI_SANDBOX_LIFETIME_MS = '600000'
 
     await withPiSandbox(async () => undefined)
 
-    expect(mockE2BCreate.mock.calls[0][1].timeoutMs).toBe(600_000)
+    expect(mockE2BCreate.mock.calls[0][1].timeoutMs).toBe(PI_SANDBOX_MIN_LIFETIME_MS)
   })
 
   it('leaves the short-lived sandbox kinds on the E2B default', async () => {

apps/sim/lib/execution/remote-sandbox/conformance.test.ts

@@ -0,0 +1,84 @@
+/**
+ * @vitest-environment node
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+/**
+ * The resolver reads configuration at import, so each case re-imports the module
+ * with its own mocked environment rather than mutating shared state.
+ */
+async function resolveWith(options: {
+  provider?: string
+  lifetimeMs?: string
+}): Promise<{ lifetime: number | undefined; min: number; max: number }> {
+  vi.resetModules()
+  vi.doMock('@/lib/core/config/env', () => ({
+    env: {
+      PI_SANDBOX_LIFETIME_MS: options.lifetimeMs,
+      SANDBOX_PROVIDER: options.provider,
+    },
+  }))
+
+  const mod = await import('@/lib/execution/remote-sandbox/pi-lifetime')
+  return {
+    lifetime: mod.resolvePiSandboxLifetimeMs(),
+    min: mod.PI_SANDBOX_MIN_LIFETIME_MS,
+    max: mod.PI_SANDBOX_MAX_LIFETIME_MS,
+  }
+}
+
+beforeEach(() => {
+  vi.resetModules()
+})
+
+describe('resolvePiSandboxLifetimeMs', () => {
+  it('defaults to the sub-hour cap on E2B', async () => {
+    const { lifetime, max } = await resolveWith({})
+
+    expect(lifetime).toBe(max)
+  })
+
+  it('has no lifetime to report when the provider stops on inactivity', async () => {
+    // Daytona has no absolute lifetime, so reporting E2B's would cut the agent
+    // turn to fit a ceiling that does not apply — the regression this prevents.
+    const { lifetime } = await resolveWith({ provider: 'daytona' })
+
+    expect(lifetime).toBeUndefined()
+  })
+
+  it('ignores a configured lifetime entirely on that provider', async () => {
+    const { lifetime } = await resolveWith({ provider: 'daytona', lifetimeMs: '600000' })
+
+    expect(lifetime).toBeUndefined()
+  })
+
+  it('lets a configured value lower the lifetime', async () => {
+    const { lifetime, min, max } = await resolveWith({ lifetimeMs: String(45 * 60 * 1000) })
+
+    expect(lifetime).toBe(45 * 60 * 1000)
+    expect(lifetime!).toBeGreaterThan(min)
+    expect(lifetime!).toBeLessThan(max)
+  })
+
+  it('refuses to be raised above the cap', async () => {
+    // A Hobby key rejects a create above one hour, so an over-large override
+    // would otherwise fail every Pi run rather than lengthening one.
+    const { lifetime, max } = await resolveWith({ lifetimeMs: String(6 * 60 * 60 * 1000) })
+
+    expect(lifetime).toBe(max)
+  })
+
+  it('raises a lifetime too short for a run to finish in', async () => {
+    // Ten minutes is consumed by the clone reserve alone, leaving the turn and
+    // the push to race a sandbox that may already be reaped.
+    const { lifetime, min } = await resolveWith({ lifetimeMs: String(10 * 60 * 1000) })
+
+    expect(lifetime).toBe(min)
+  })
+
+  it.each(['', 'soon', '0', '-1'])('falls back to the cap for %o', async (value) => {
+    const { lifetime, max } = await resolveWith({ lifetimeMs: value })
+
+    expect(lifetime).toBe(max)
+  })
+})

apps/sim/lib/execution/remote-sandbox/pi-lifetime.test.ts

@@ -4,8 +4,20 @@
  * against the same number without importing the provider SDKs.
  */
 
+import { createLogger } from '@sim/logger'
 import { env } from '@/lib/core/config/env'
 
+const logger = createLogger('PiSandboxLifetime')
+
+/**
+ * Read from `env` rather than the `env-flags` gate, and normalized the same way
+ * `remote-sandbox/index.ts` normalizes it, so this module keeps the independence
+ * its header describes: no provider adapters, no barrel, no config gate.
+ */
+function isLifetimeProvider(): boolean {
+  return (env.SANDBOX_PROVIDER?.toLowerCase() ?? 'e2b') === 'e2b'
+}
+
 /**
  * E2B documents a one-hour maximum sandbox lifetime for Hobby accounts (24 hours
  * for Pro) and rejects a create above it. The cap sits strictly below that hour:
@@ -15,20 +27,54 @@ import { env } from '@/lib/core/config/env'
 export const PI_SANDBOX_MAX_LIFETIME_MS = 59 * 60 * 1000
 
 /**
- * The lifetime requested for a Pi sandbox, always clamped to
- * {@link PI_SANDBOX_MAX_LIFETIME_MS}. Defaults to the cap: a run that finishes
- * kills its sandbox explicitly, so for the normal path the lifetime is a ceiling
+ * The shortest lifetime a Pi run can actually complete in, and the floor an
+ * override is raised to.
+ *
+ * A Pi backend brackets the agent turn with a clone and two finalize commands,
+ * and caps the turn itself against whatever is left. Below this, that arithmetic
+ * has no positive remainder: the reserves alone exhaust the lifetime, so E2B
+ * could reap the sandbox before the turn or the push finished. Raising the value
+ * is better than rejecting it — a module-scope throw on a config typo would take
+ * down every execution path that imports this, not just Pi.
+ *
+ * `cloud-shared.test.ts` asserts this stays at or above the backends' own
+ * reserves, so the two cannot drift apart silently.
+ */
+export const PI_SANDBOX_MIN_LIFETIME_MS = 31 * 60 * 1000
+
+/**
+ * The lifetime requested for a Pi sandbox, or `undefined` when the selected
+ * provider has no such concept.
+ *
+ * Only E2B takes an absolute lifetime. Daytona stops on inactivity instead
+ * (`autoStopInterval`, refreshed by activity), so there is no ceiling for a Pi
+ * command to reserve against — and deriving one anyway would shorten Daytona's
+ * agent turn to fit a limit that does not apply to it.
+ *
+ * For E2B it defaults to {@link PI_SANDBOX_MAX_LIFETIME_MS}: a run that finishes
+ * kills its sandbox explicitly, so on the normal path the lifetime is a ceiling
  * rather than a budget. It is not entirely free — if the web process dies
  * mid-run the orphaned sandbox now bills until this ceiling instead of the SDK's
  * five minutes — but five minutes is short enough to kill live runs, which is
  * the bug this replaces.
  *
- * `PI_SANDBOX_LIFETIME_MS` lowers it (a Pro account can raise the constant, but
- * the env var may only reduce it, so a misconfigured value cannot make every
- * create fail on a Hobby key).
+ * `PI_SANDBOX_LIFETIME_MS` may only lower it, and only as far as
+ * {@link PI_SANDBOX_MIN_LIFETIME_MS}, so a misconfigured value can neither make
+ * every create fail on a Hobby key nor leave a run without time to push.
  */
-export function resolvePiSandboxLifetimeMs(): number {
+export function resolvePiSandboxLifetimeMs(): number | undefined {
+  if (!isLifetimeProvider()) return undefined
+
   const configured = Number.parseInt(env.PI_SANDBOX_LIFETIME_MS ?? '', 10)
   if (!Number.isFinite(configured) || configured <= 0) return PI_SANDBOX_MAX_LIFETIME_MS
+
+  if (configured < PI_SANDBOX_MIN_LIFETIME_MS) {
+    logger.warn('PI_SANDBOX_LIFETIME_MS is below the minimum a Pi run can finish in; raising it', {
+      configured,
+      using: PI_SANDBOX_MIN_LIFETIME_MS,
+    })
+    return PI_SANDBOX_MIN_LIFETIME_MS
+  }
+
   return Math.min(configured, PI_SANDBOX_MAX_LIFETIME_MS)
 }

apps/sim/lib/execution/remote-sandbox/pi-lifetime.ts

@@ -23,6 +23,28 @@ describe('github_job_logs', () => {
     expect(url).toBe('https://api.github.com/repos/octo/demo/actions/jobs/42/logs')
   })
 
+  it('escapes coordinates so they cannot redirect the authenticated request', () => {
+    const url = (jobLogsTool.request.url as (params: JobLogsParams) => string)({
+      ...BASE_PARAMS,
+      owner: '../../orgs/secret',
+      repo: 'demo?ref=x',
+    })
+
+    expect(url).toBe(
+      'https://api.github.com/repos/..%2F..%2Forgs%2Fsecret/demo%3Fref%3Dx/actions/jobs/42/logs'
+    )
+  })
+
+  it('rejects a job id that is not a positive integer', () => {
+    const url = jobLogsTool.request.url as (params: JobLogsParams) => string
+
+    expect(() => url({ ...BASE_PARAMS, job_id: 0 })).toThrow(/job_id must be a positive integer/)
+    expect(() => url({ ...BASE_PARAMS, job_id: 1.5 })).toThrow(/job_id must be a positive integer/)
+    expect(() => url({ ...BASE_PARAMS, job_id: '9/../..' as unknown as number })).toThrow(
+      /job_id must be a positive integer/
+    )
+  })
+
   it('returns a short log whole', async () => {
     const result = await jobLogsTool.transformResponse!(logResponse('boom\n'), BASE_PARAMS)
 

apps/sim/tools/github/job_logs.test.ts

@@ -12,6 +12,22 @@ function resolveMaxCharacters(value: number | undefined): number {
   return requested
 }
 
+/**
+ * Every path segment is escaped or checked before it reaches the URL.
+ *
+ * Raw interpolation is the prevailing shape among the GitHub tools here, but it
+ * costs more in this one: the response body is returned verbatim as `logs`
+ * instead of being parsed into a fixed shape, so a coordinate carrying URL syntax
+ * would turn a bearer-authenticated request into a general read of whatever
+ * endpoint it reached. Siblings that parse a typed response fail closed instead.
+ */
+function jobLogsPath(owner: string, repo: string, jobId: number): string {
+  if (!Number.isSafeInteger(jobId) || jobId < 1) {
+    throw new Error('job_id must be a positive integer')
+  }
+  return `${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/actions/jobs/${jobId}/logs`
+}
+
 /**
  * The tail is what matters: a failing job reports its error at the end.
  *
@@ -76,7 +92,7 @@ export const jobLogsTool: ToolConfig<JobLogsParams, JobLogsResponse> = {
     // The per-job endpoint, not the run-level zip archive. GitHub answers with a
     // 302 to a short-lived blob URL that carries its own signature.
     url: (params) =>
-      `https://api.github.com/repos/${params.owner}/${params.repo}/actions/jobs/${params.job_id}/logs`,
+      `https://api.github.com/repos/${jobLogsPath(params.owner, params.repo, params.job_id)}`,
     method: 'GET',
     headers: (params) => ({
       Accept: 'application/vnd.github+json',