perf: throttle CLI screenshot cleanup (#4089)

src/cli/format.test.ts

@@ -1,13 +1,28 @@
-import { describe, expect, it } from 'vitest'
+import { existsSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from 'fs'
+import { tmpdir } from 'os'
+import { join } from 'path'
+import { afterEach, describe, expect, it, vi } from 'vitest'
 import { RuntimeRpcFailureError } from './runtime-client'
 import {
   formatCliError,
   formatComputerAction,
   formatTerminalRead,
-  formatWorktreeList
+  formatWorktreeList,
+  printResult
 } from './format'
 import type { ComputerActionResult, RuntimeWorktreeRecord } from '../shared/runtime-types'
 
+let testScreenshotDir: string | null = null
+
+afterEach(() => {
+  vi.restoreAllMocks()
+  delete process.env.ORCA_COMPUTER_SCREENSHOT_TMPDIR
+  if (testScreenshotDir) {
+    rmSync(testScreenshotDir, { recursive: true, force: true })
+    testScreenshotDir = null
+  }
+})
+
 function worktree(overrides: Partial<RuntimeWorktreeRecord> = {}): RuntimeWorktreeRecord {
   const base: RuntimeWorktreeRecord = {
     id: 'repo::/tmp/repo/child',
@@ -235,3 +250,75 @@ describe('formatComputerAction', () => {
     )
   })
 })
+
+describe('printResult computer screenshots', () => {
+  it('removes expired screenshot temp files when cleanup is due', () => {
+    testScreenshotDir = mkdtempSync(join(tmpdir(), 'orca-format-test-'))
+    process.env.ORCA_COMPUTER_SCREENSHOT_TMPDIR = testScreenshotDir
+    const expiredPath = join(testScreenshotDir, 'old-screenshot.png')
+    writeFileSync(expiredPath, 'old')
+    const expired = new Date(Date.now() - 48 * 60 * 60 * 1000)
+    utimesSync(expiredPath, expired, expired)
+    const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined)
+
+    printResult(
+      {
+        id: 'req-cleanup',
+        ok: true,
+        result: {
+          screenshot: {
+            data: Buffer.from('png-data').toString('base64'),
+            format: 'png',
+            width: 1,
+            height: 1,
+            scale: 1
+          }
+        },
+        _meta: { runtimeId: 'runtime-1' }
+      },
+      true,
+      () => 'unused'
+    )
+
+    expect(existsSync(expiredPath)).toBe(false)
+    expect(existsSync(join(testScreenshotDir, '.last-cleanup'))).toBe(true)
+    expect(logSpy).toHaveBeenCalled()
+  })
+
+  it('skips screenshot temp cleanup when the cleanup marker is fresh', () => {
+    testScreenshotDir = mkdtempSync(join(tmpdir(), 'orca-format-test-'))
+    const expiredPath = join(testScreenshotDir, 'old-screenshot.png')
+    writeFileSync(expiredPath, 'old')
+    const expired = new Date(Date.now() - 48 * 60 * 60 * 1000)
+    utimesSync(expiredPath, expired, expired)
+    writeFileSync(join(testScreenshotDir, '.last-cleanup'), 'recent\n')
+    process.env.ORCA_COMPUTER_SCREENSHOT_TMPDIR = testScreenshotDir
+    const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined)
+
+    printResult(
+      {
+        id: 'req/1',
+        ok: true,
+        result: {
+          screenshot: {
+            data: Buffer.from('png-data').toString('base64'),
+            format: 'png',
+            width: 1,
+            height: 1,
+            scale: 1
+          }
+        },
+        _meta: { runtimeId: 'runtime-1' }
+      },
+      true,
+      () => 'unused'
+    )
+
+    expect(existsSync(expiredPath)).toBe(true)
+    const output = JSON.parse(logSpy.mock.calls[0][0]) as {
+      result: { screenshot: { dataOmitted: boolean; path: string } }
+    }
+    expect(output.result.screenshot.dataOmitted).toBe(true)
+    expect(output.result.screenshot.path).toContain('req_1-screenshot.png')
+  })
+})

src/cli/format.ts

@@ -511,9 +511,12 @@ function prepareCliJsonResult<TResult>(
 }
 
 const COMPUTER_SCREENSHOT_TTL_MS = 24 * 60 * 60 * 1000
+const COMPUTER_SCREENSHOT_CLEANUP_INTERVAL_MS = 60 * 60 * 1000
+const COMPUTER_SCREENSHOT_CLEANUP_MARKER = '.last-cleanup'
 
 function computerScreenshotTempDir(): string {
-  const outputDir = join(tmpdir(), 'orca-computer-use')
+  const outputDir =
+    process.env.ORCA_COMPUTER_SCREENSHOT_TMPDIR || join(tmpdir(), 'orca-computer-use')
   mkdirSync(outputDir, { recursive: true, mode: 0o700 })
   const stat = lstatSync(outputDir)
   if (!stat.isDirectory() || stat.isSymbolicLink()) {
@@ -527,7 +530,19 @@ function computerScreenshotTempDir(): string {
 }
 
 function cleanupComputerScreenshots(outputDir: string): void {
-  const cutoff = Date.now() - COMPUTER_SCREENSHOT_TTL_MS
+  const now = Date.now()
+  const markerPath = join(outputDir, COMPUTER_SCREENSHOT_CLEANUP_MARKER)
+  try {
+    // Why: agents can call computer-use CLI commands in loops; a marker keeps
+    // temp cleanup from becoming a synchronous directory scan per screenshot.
+    if (statSync(markerPath).mtimeMs > now - COMPUTER_SCREENSHOT_CLEANUP_INTERVAL_MS) {
+      return
+    }
+  } catch {
+    // Missing or unreadable marker means this process should attempt cleanup.
+  }
+
+  const cutoff = now - COMPUTER_SCREENSHOT_TTL_MS
   for (const entry of readdirSync(outputDir)) {
     if (!entry.endsWith('-screenshot.png') && !entry.endsWith('-screenshot.img')) {
       continue
@@ -541,6 +556,11 @@ function cleanupComputerScreenshots(outputDir: string): void {
       // Best-effort cleanup only; formatting should not fail because a temp file raced.
     }
   }
+  try {
+    writeFileSync(markerPath, `${now}\n`, { mode: 0o600 })
+  } catch {
+    // Best-effort marker only; stale cleanup state should not hide a screenshot.
+  }
 }
 
 function safeCliFileStem(value: string): string {