fix(orchestration): wait for Codex composer render (#14575)

2026-09-22 08:02:28 +00:00 · 2026-08-14 12:56:35 -07:00
parent ebb60a0757
commit ff9bc0f079
3 changed files with 152 additions and 84 deletions
@@ -8645,10 +8645,14 @@
      "providers": ["local", "daemon", "ssh", "remote-runtime"],
      "coveredPlatforms": ["macos"],
      "coveredProviders": ["local"],
-      "coverageNotes": "Local macOS evidence covers the runtime PTY write contract, a live dev-runtime CLI repro, and a Haiku composer trace. The Claude render gate stays on the PTY-owning runtime, so paired and SSH callers use the same provider write/output path without a wire change. SSH, daemon, remote-runtime, Linux, and Windows live journeys remain gaps.",
-      "motivatingLinks": ["https://github.com/stablyai/orca/issues/7226"],
-      "invariant": "Injected orchestration task prompts for recognized agent CLIs must send the prompt body inside one bracketed-paste frame, sanitize embedded ESC bytes, preserve chunk boundaries without losing the frame, and submit exactly once only after the agent can accept Enter. Claude must emit a post-paste composer marker and then settle, or reach the bounded fallback first.",
-      "oracle": "Runtime tests assert the exact PTY write sequence, failure cleanup, and a Claude-shaped multi-frame render where the legacy 500 ms delay and first show-cursor marker are both premature. The candidate waits for post-marker output quiescence before one submit, resets that window on later frames, and still submits once at the hard deadline if output never settles. Orchestration tests assert dispatch/coordinator use the agent prompt path; the live CLI harness covers long Codex-like framing.",
+      "coverageNotes": "Local macOS evidence covers the runtime PTY write contract, live Codex orchestration, a dev-runtime CLI repro, and Claude and Codex composer traces. The render gate stays on the PTY-owning runtime, so paired and SSH callers use the same provider write/output path without a wire change. SSH, daemon, remote-runtime, Linux, and Windows live journeys remain gaps.",
+      "motivatingLinks": [
+        "https://github.com/stablyai/orca/issues/7226",
+        "https://github.com/stablyai/orca/issues/13821",
+        "https://github.com/stablyai/orca/issues/14347"
+      ],
+      "invariant": "Injected orchestration task prompts for recognized agent CLIs must send the prompt body inside one bracketed-paste frame, sanitize embedded ESC bytes, preserve chunk boundaries without losing the frame, and submit exactly once only after the agent can accept Enter. Claude and Codex must emit a post-paste composer marker and then settle, or reach the bounded fallback first; a late marker receives a fresh settlement window.",
+      "oracle": "Runtime tests assert the exact PTY write sequence, failure cleanup, and Claude- and Codex-shaped multi-frame renders where the legacy platform delay and first show-cursor marker are both premature. The candidate waits for post-marker output quiescence before one submit, resets that window on later frames, gives a late marker a fresh bounded window, and still submits once at the hard deadline if output never settles. Orchestration tests assert dispatch/coordinator use the agent prompt path; the live CLI harness covers long Codex-like framing.",
      "commands": [
        "pnpm exec vitest run --config config/vitest.config.ts src/shared/agent-prompt-injection.test.ts src/main/runtime/orca-runtime.test.ts src/main/runtime/rpc/methods/orchestration.test.ts src/main/runtime/orchestration/coordinator.test.ts",
        "node tests/tools/repro-orchestration-long-prompt.mjs --cli out/bin/orca-dev --mode codex-like --size-kb 32 --timeout-ms 20000"
@@ -8674,7 +8678,8 @@
          "file": "src/main/runtime/orca-runtime.test.ts",
          "assertions": [
            "runtime writes bracketed paste before a delayed submit",
-            "Claude ignores unrelated and split pre-marker output, then waits for post-marker render quiescence before one submit",
+            "Claude and Codex ignore unrelated and split pre-marker output, then wait for post-marker render quiescence before one submit",
+            "a late Codex marker receives a fresh settlement window before one submit",
            "a silent Claude composer reaches one bounded fallback submit",
            "a marked Claude composer with continuous render output reaches one bounded fallback submit",
            "large prompt frames are chunked and reconstructed before submit",
@@ -8706,6 +8711,15 @@
        }
      ],
      "evidenceRuns": [
+        {
+          "date": "2026-08-14",
+          "runner": "local",
+          "platform": "macos",
+          "command": "pnpm exec vitest run --config config/vitest.config.ts src/shared/agent-prompt-injection.test.ts src/main/runtime/orca-runtime.test.ts src/main/runtime/rpc/methods/orchestration.test.ts src/main/runtime/orchestration/coordinator.test.ts",
+          "result": "passed",
+          "durationSeconds": 11.32,
+          "summary": "4 files and 1,303 tests passed with one skipped. Claude and Codex both wait for post-marker quiescence, and a Codex marker arriving at 7.9 seconds receives a fresh window through its final slow frame. Exact-build live Codex workers accepted injected prompts without manual Enter, replied, called worker_done, and settled successfully in the rendered Electron UI."
+        },
        {
          "date": "2026-08-13",
          "runner": "local",
@@ -8753,11 +8767,11 @@
      },
      "redGreenEvidence": {
        "status": "partial",
-        "evidence": "The original Claude-shaped oracle fails on unmodified main and passes with the render gate. The hardened multi-frame oracle then fails on the first-marker candidate, which submits at 751 ms during an intermediate frame, and passes when the gate requires post-marker quiescence. The earlier live harness also reproduced the unsafe raw multiline contract before its framing fix. Intentional-break evidence remains local rather than a separate CI job."
+        "evidence": "The original Claude-shaped oracle fails on unmodified main and passes with the render gate. The hardened multi-frame oracle then fails on the first-marker candidate, which submits at 751 ms during an intermediate frame, and passes when the gate requires post-marker quiescence. Codex used the legacy platform delay before this candidate; the late-marker oracle proves the original deadline would pre-empt slow-host settlement without re-arming. The earlier live harness also reproduced the unsafe raw multiline contract before its framing fix. Intentional-break evidence remains local rather than a separate CI job."
      },
      "performanceBudget": {
        "required": true,
-        "evidence": "Agent prompt dispatch remains O(prompt bytes) with existing 16KB chunking. Claude adds one PTY-local output listener until 1.5s of post-marker output quiescence or an 8s fallback; each later frame resets one timer. Other agents retain the platform delay. No polling, provider listing, subprocess churn, hidden-pane wakeups, renderer work, or wire traffic is added."
+        "evidence": "Agent prompt dispatch remains O(prompt bytes) with existing 16KB chunking. Claude and Codex add one PTY-local output listener until 1.5s of post-marker output quiescence or an 8s fallback; a late first marker restarts the bounded window and each later frame resets one quiet timer. Other agents retain the platform delay. No polling, provider listing, subprocess churn, hidden-pane wakeups, renderer work, or wire traffic is added."
      },
      "promotionCriteria": [
        "Run the live harness in soak with a self-starting dev runtime or provider-contract fixture.",
@@ -16630,75 +16630,78 @@ describe('OrcaRuntimeService', () => {
    }
  })

-  it('waits for Claude output to settle after its first render marker before one submit', async () => {
-    vi.useFakeTimers()
-    try {
-      const writes: string[] = []
-      let composerReady = false
-      let prematureEnters = 0
-      let submissions = 0
-      const runtime = new OrcaRuntimeService(store)
-      runtime.setPtyController({
-        spawn: vi.fn().mockResolvedValue({ id: 'pty-bg' }),
-        write: (_ptyId, data) => {
-          writes.push(data)
-          if (data.includes(AGENT_PROMPT_BRACKETED_PASTE_END)) {
-            setTimeout(() => {
-              runtime.onPtyData('pty-bg', 'partial redraw without cursor', Date.now())
-            }, 650)
-            setTimeout(() => {
-              runtime.onPtyData('pty-bg', '\x1b[?2', Date.now())
-            }, 750)
-            setTimeout(() => {
-              runtime.onPtyData('pty-bg', '5h intermediate frame', Date.now())
-            }, 751)
-            setTimeout(() => {
-              runtime.onPtyData('pty-bg', 'continued composer render', Date.now())
-            }, 900)
-            setTimeout(() => {
-              composerReady = true
-              runtime.onPtyData('pty-bg', 'final composer frame', Date.now())
-            }, 1_000)
-          }
-          if (data === '\r') {
-            if (composerReady) {
-              submissions += 1
-            } else {
-              prematureEnters += 1
+  it.each(['claude', 'codex'] as const)(
+    'waits for %s output to settle after its first render marker before one submit',
+    async (agent) => {
+      vi.useFakeTimers()
+      try {
+        const writes: string[] = []
+        let composerReady = false
+        let prematureEnters = 0
+        let submissions = 0
+        const runtime = new OrcaRuntimeService(store)
+        runtime.setPtyController({
+          spawn: vi.fn().mockResolvedValue({ id: 'pty-bg' }),
+          write: (_ptyId, data) => {
+            writes.push(data)
+            if (data.includes(AGENT_PROMPT_BRACKETED_PASTE_END)) {
+              setTimeout(() => {
+                runtime.onPtyData('pty-bg', 'partial redraw without cursor', Date.now())
+              }, 650)
+              setTimeout(() => {
+                runtime.onPtyData('pty-bg', '\x1b[?2', Date.now())
+              }, 750)
+              setTimeout(() => {
+                runtime.onPtyData('pty-bg', '5h intermediate frame', Date.now())
+              }, 751)
+              setTimeout(() => {
+                runtime.onPtyData('pty-bg', 'continued composer render', Date.now())
+              }, 900)
+              setTimeout(() => {
+                composerReady = true
+                runtime.onPtyData('pty-bg', 'final composer frame', Date.now())
+              }, 1_000)
            }
-          }
-          return true
-        },
-        kill: () => true,
-        getForegroundProcess: async () => null
-      })
-      const { handle } = await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, {
-        launchAgent: 'claude'
-      })
-      const assertAuthority = vi.fn()
+            if (data === '\r') {
+              if (composerReady) {
+                submissions += 1
+              } else {
+                prematureEnters += 1
+              }
+            }
+            return true
+          },
+          kill: () => true,
+          getForegroundProcess: async () => null
+        })
+        const { handle } = await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, {
+          launchAgent: agent
+        })
+        const assertAuthority = vi.fn()

-      const sendPromise = runtime.sendTerminalAgentPrompt(handle, 'review this change', {
-        beforeWrite: assertAuthority
-      })
-      await vi.advanceTimersByTimeAsync(500)
+        const sendPromise = runtime.sendTerminalAgentPrompt(handle, 'review this change', {
+          beforeWrite: assertAuthority
+        })
+        await vi.advanceTimersByTimeAsync(500)

-      expect(writes).not.toContain('\r')
-      await vi.advanceTimersByTimeAsync(150)
-      expect(writes).not.toContain('\r')
-      await vi.advanceTimersByTimeAsync(101)
-      expect(writes).not.toContain('\r')
-      await vi.advanceTimersByTimeAsync(1_748)
-      expect(writes).not.toContain('\r')
-      await vi.advanceTimersByTimeAsync(1)
-      await sendPromise
-      expect(prematureEnters).toBe(0)
-      expect(submissions).toBe(1)
-      expect(writes.filter((data) => data === '\r')).toHaveLength(1)
-      expect(assertAuthority).toHaveBeenCalledTimes(2)
-    } finally {
-      vi.useRealTimers()
+        expect(writes).not.toContain('\r')
+        await vi.advanceTimersByTimeAsync(150)
+        expect(writes).not.toContain('\r')
+        await vi.advanceTimersByTimeAsync(101)
+        expect(writes).not.toContain('\r')
+        await vi.advanceTimersByTimeAsync(1_748)
+        expect(writes).not.toContain('\r')
+        await vi.advanceTimersByTimeAsync(1)
+        await sendPromise
+        expect(prematureEnters).toBe(0)
+        expect(submissions).toBe(1)
+        expect(writes.filter((data) => data === '\r')).toHaveLength(1)
+        expect(assertAuthority).toHaveBeenCalledTimes(2)
+      } finally {
+        vi.useRealTimers()
+      }
    }
-  })
+  )

  it('submits a silent Claude composer once after the bounded render fallback', async () => {
    vi.useFakeTimers()
@@ -16730,6 +16733,46 @@ describe('OrcaRuntimeService', () => {
    }
  })

+  it('gives a late Codex render marker a fresh quiescence window', async () => {
+    vi.useFakeTimers()
+    try {
+      const writes: string[] = []
+      let composerReady = false
+      const runtime = new OrcaRuntimeService(store)
+      runtime.setPtyController({
+        spawn: vi.fn().mockResolvedValue({ id: 'pty-bg' }),
+        write: (_ptyId, data) => {
+          writes.push(data)
+          if (data.includes(AGENT_PROMPT_BRACKETED_PASTE_END)) {
+            setTimeout(() => runtime.onPtyData('pty-bg', '\x1b[?25h', Date.now()), 7_900)
+            setTimeout(() => {
+              composerReady = true
+              runtime.onPtyData('pty-bg', 'final slow composer frame', Date.now())
+            }, 8_100)
+          }
+          return true
+        },
+        kill: () => true,
+        getForegroundProcess: async () => null
+      })
+      const { handle } = await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, {
+        launchAgent: 'codex'
+      })
+
+      const sendPromise = runtime.sendTerminalAgentPrompt(handle, 'review this change')
+      await vi.advanceTimersByTimeAsync(8_000)
+      expect(writes).not.toContain('\r')
+      await vi.advanceTimersByTimeAsync(1_599)
+      expect(writes).not.toContain('\r')
+      await vi.advanceTimersByTimeAsync(1)
+      await sendPromise
+      expect(composerReady).toBe(true)
+      expect(writes.filter((data) => data === '\r')).toHaveLength(1)
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
  it('bounds a Claude render that never settles to one fallback submit', async () => {
    vi.useFakeTimers()
    try {
@@ -16758,7 +16801,7 @@ describe('OrcaRuntimeService', () => {
      })

      const sendPromise = runtime.sendTerminalAgentPrompt(handle, 'review this change')
-      await vi.advanceTimersByTimeAsync(7_999)
+      await vi.advanceTimersByTimeAsync(8_099)
      expect(writes).not.toContain('\r')

      await vi.advanceTimersByTimeAsync(1)
@@ -1864,10 +1864,10 @@ const BRACKETED_PASTE_BEGIN = '\x1b[200~'
 const BRACKETED_PASTE_END = '\x1b[201~'
 const BRACKETED_PASTE_QUIET_MS = 1500
 const DRAFT_PASTE_READY_TIMEOUT_MS = 8000
-const CLAUDE_AGENT_PROMPT_RENDER_TIMEOUT_MS = 8000
-const CLAUDE_AGENT_PROMPT_RENDER_QUIET_MS = 1500
-// Why: Claude emits show-cursor while rendering its composer; output must settle afterward.
-const CLAUDE_AGENT_PROMPT_RENDER_MARKER = '\x1b[?25h'
+const AGENT_PROMPT_RENDER_TIMEOUT_MS = 8000
+const AGENT_PROMPT_RENDER_QUIET_MS = 1500
+// Why: Claude and Codex emit show-cursor while rendering pasted composer content.
+const AGENT_PROMPT_RENDER_MARKER = '\x1b[?25h'
 const MOBILE_TERMINAL_SURFACE_TIMEOUT_MS = 10_000
 // Why: the split already failed; the caller waits on this teardown only to learn whether the
 // fallback kill is needed, so keep it short — an unreachable host must not stall the rejection.
@@ -17557,7 +17557,7 @@ export class OrcaRuntimeService {
      suffixFailureError?: string
    } = {}
  ): Promise<void> {
-    const renderGate = this.createClaudeAgentPromptRenderGate(ptyId)
+    const renderGate = this.createAgentPromptRenderGate(ptyId)
    let wrotePasteBytes = false
    let completedPaste = false
    try {
@@ -17608,13 +17608,14 @@ export class OrcaRuntimeService {
    }
  }

-  private createClaudeAgentPromptRenderGate(ptyId: string): {
+  private createAgentPromptRenderGate(ptyId: string): {
    arm: () => void
    wait: () => Promise<void>
    dispose: () => void
  } | null {
    const pty = this.ptysById.get(ptyId)
-    if ((pty?.launchAgent ?? pty?.foregroundAgent) !== 'claude') {
+    const agent = pty?.launchAgent ?? pty?.foregroundAgent
+    if (agent !== 'claude' && agent !== 'codex') {
      return null
    }
    let armed = false
@@ -17647,7 +17648,13 @@ export class OrcaRuntimeService {
      if (quietTimer) {
        clearTimeout(quietTimer)
      }
-      quietTimer = setTimeout(finish, CLAUDE_AGENT_PROMPT_RENDER_QUIET_MS)
+      quietTimer = setTimeout(finish, AGENT_PROMPT_RENDER_QUIET_MS)
+    }
+    const armHardTimer = (): void => {
+      if (hardTimer) {
+        clearTimeout(hardTimer)
+      }
+      hardTimer = setTimeout(finish, AGENT_PROMPT_RENDER_TIMEOUT_MS)
    }
    const unsubscribe = this.subscribeToTerminalData(ptyId, (data) => {
      if (!armed || settled) {
@@ -17655,11 +17662,13 @@ export class OrcaRuntimeService {
      }
      if (!observedMarker) {
        const combined = markerCarry + data
-        markerCarry = combined.slice(-(CLAUDE_AGENT_PROMPT_RENDER_MARKER.length - 1))
-        if (!combined.includes(CLAUDE_AGENT_PROMPT_RENDER_MARKER)) {
+        markerCarry = combined.slice(-(AGENT_PROMPT_RENDER_MARKER.length - 1))
+        if (!combined.includes(AGENT_PROMPT_RENDER_MARKER)) {
          return
        }
        observedMarker = true
+        // Why: a slow initial redraw must still receive the full settlement window.
+        armHardTimer()
      }
      armQuietTimer()
    })
@@ -17672,7 +17681,9 @@ export class OrcaRuntimeService {
        if (settled) {
          return
        }
-        hardTimer = setTimeout(finish, CLAUDE_AGENT_PROMPT_RENDER_TIMEOUT_MS)
+        if (!hardTimer) {
+          armHardTimer()
+        }
        await rendered
      },
      dispose: () => {