Skip to content

fix(agent-status): clear answered Claude and Codex waits#8383

Open
gatsby74 wants to merge 3 commits into
stablyai:mainfrom
gatsby74:fix/askuserquestion-permission-wait-stuck
Open

fix(agent-status): clear answered Claude and Codex waits#8383
gatsby74 wants to merge 3 commits into
stablyai:mainfrom
gatsby74:fix/askuserquestion-permission-wait-stuck

Conversation

@gatsby74

@gatsby74 gatsby74 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes two related agent-status waits that stayed amber after the user answered:

  • Claude AskUserQuestion can emit a trailing PermissionRequest without a tool_use_id, so Orca could not match the answer to the waiting tool.
  • Codex command approval emits PermissionRequest, but Codex 0.144.1 emits no execution-start hook after approval. Its next hook is PostToolUse, after the command finishes, so the row stayed waiting throughout execution.

Fix

  • Treat Claude AskUserQuestion permission events as interactive questions so the next working hook clears the wait.
  • When the main runtime observes the transition into a working terminal title for a fresh Codex PermissionRequest, synthesize the missing waiting-to-working transition in the hook-status owner.
  • Clear the interactive prompt while preserving prompt, tool, routing, and provider-session metadata.
  • Keep the Codex fallback narrowly gated to fresh Codex PermissionRequest waits; other agents and states are unchanged.

The Codex transition lives in the main runtime and hook server, so desktop, parked terminals, worktree ps, headless serve, and SSH/remote runtimes read the same corrected state.

Root cause

Claude and Codex expose different incomplete lifecycle sequences around user input. Claude supplies an answer hook but its preceding AskUserQuestion permission event lacks the identifier used by the sticky-permission guard. Codex supplies the permission hook but no hook when approval resumes execution. In both cases Orca retained a valid waiting event after the terminal had already resumed.

Screenshots

No layout or styling change. The visible behavior changes from an amber waiting dot remaining during command execution to the normal working indicator immediately after approval.

Testing

  • pnpm run lint
  • pnpm run typecheck:web
  • pnpm run check:max-lines-ratchet
  • 668 targeted tests across the agent-hook server and main runtime
  • Formatting and git diff --check
  • Live dev-app reproduction with this worktree CLI and Codex 0.144.1

Live Codex validation used an on-request session and an approved sleep 15 command:

  1. Before approval: terminal title was Action Required; worktree ps reported waiting.
  2. Immediately after approval: ten consecutive samples during the still-running command reported working.
  3. After PostToolUse and completion: the row moved to done with MAIN_FIX_COMPLETE.

AI Review Report

CodeRabbit review feedback was applied:

  • The resume callback now runs only on the transition into working, not on every spinner frame.
  • Regression coverage now includes stale waits and Codex waits not originating from PermissionRequest.
  • The runtime routing test was moved into a dedicated describe block and asserts spinner frames do not retrigger the callback.
  • New production and test helpers include focused documentation.

No conflicting reviewer guidance or unresolved inline review threads remain.

Security Audit

No new command execution, authentication, filesystem access, dependency, or public IPC surface was added. The fallback only transforms an already-normalized, fresh Codex status row after a pane-scoped runtime title transition. Existing payload caps and pane-key validation remain in force.

Compatibility and risk

  • Cross-platform title classification and pane routing are reused.
  • The runtime path is shared by local and serve-mode hosts, covering SSH without local-only assumptions.
  • Genuine Claude tool permission gates remain sticky; the existing regression test continues to cover that behavior.
  • Spinner animation frames are transition-gated and do not repeatedly enter the hook server.

Notes

  • Codex 0.144.1 was used for the live reproduction and verification.
  • This extends the original Claude-focused fix in the same PR because both symptoms come from missing provider lifecycle information around answered prompts.
  • No visual assets or localization changes are required.

…ionRequest

Claude's AskUserQuestion is auto-allowed, but in addition to the PreToolUse
hook it emits a PermissionRequest ~120ms later. Orca registers
PermissionRequest, so that event overwrites the PreToolUse as the cached
`previous` status — and it carries no tool_use_id.

shouldKeepClaudePermissionVisible (from stablyai#8311) keeps a Claude
PermissionRequest wait sticky until a resuming-tool id match clears it. Since
the AskUserQuestion PermissionRequest exposes no tool_use_id, the answer's
PostToolUse 'working' hook can never satisfy that match, so the sidebar row
stays pinned on the amber "waiting" attention state until the turn's Stop
flips it straight to done. Users see the row appear stuck from the moment
they answer until the whole reply finishes.

stablyai#8311 only covered the PreToolUse-attributed wait; the PermissionRequest that
lands right after it slips through. Treat any PermissionRequest whose tool is
an AskUserQuestion variant as an interactive question rather than an
Allow/Deny gate, so its wait clears on the next 'working' hook. Real tool
permission requests (Bash, etc.) stay sticky as before.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SCLYfTSDnDyBgotAp8yRoV
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Exports isAskUserQuestionTool and updates Claude hook handling so AskUserQuestion permission states clear when a subsequent PostToolUse event reports working. Adds Codex permission-wait resumption from terminal-title status updates, including fresh-state validation, pane-key resolution, server status updates, core-handler wiring, and runtime callback handling. Tests cover both Claude state transitions and Codex resume behavior.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely summarizes the main change: clearing answered Claude and Codex waits.
Description check ✅ Passed All required sections are present and substantive, but the AI Review Report should explicitly name macOS, Linux, and Windows checks.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gatsby74 gatsby74 changed the title fix(agent-status): clear AskUserQuestion waits that arrive as PermissionRequest fix(agent-status): clear answered Claude and Codex waits Jul 12, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (3)
src/main/agent-hooks/server.codex-permission-resume.test.ts (1)

28-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add tests for untested guard conditions in resumeCodexPermissionWaitFromTerminalTitle.

Two of the four guard conditions in the resume method are not exercised: the staleness check (Date.now() - existing.receivedAt > AGENT_STATUS_STALE_AFTER_MS) and the hookEventName !== 'PermissionRequest' check for a Codex waiting entry. Adding these would confirm the guards reject stale and non-permission waits.

🧪 Suggested additional test cases
 import { describe, expect, it, vi } from 'vitest'
 import { makePaneKey } from '../../shared/stable-pane-id'
+import { AGENT_STATUS_STALE_AFTER_MS } from '../../shared/agent-status-types'
 import { AgentHookServer } from './server'
   it('ignores other agents and Codex states that are not permission waits', () => {
     // ... existing test body ...
   })
+
+  it('ignores stale Codex permission waits', () => {
+    vi.useFakeTimers()
+    const server = new AgentHookServer()
+    ingestCodexPermission(server)
+    vi.advanceTimersByTime(AGENT_STATUS_STALE_AFTER_MS + 1)
+    expect(server.resumeCodexPermissionWaitFromTerminalTitle(PANE_KEY)).toBe(false)
+    vi.useRealTimers()
+  })
+
+  it('ignores Codex waiting states without a PermissionRequest hook event', () => {
+    const server = new AgentHookServer()
+    server.ingestRemote(
+      {
+        paneKey: PANE_KEY,
+        hookEventName: 'AskUserQuestion',
+        payload: { state: 'waiting', prompt: 'Answer me', agentType: 'codex' }
+      },
+      'connection-1'
+    )
+    expect(server.resumeCodexPermissionWaitFromTerminalTitle(PANE_KEY)).toBe(false)
+  })
src/main/runtime/orca-runtime.ts (1)

6207-6210: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Resume-wait hook fires on every "working" title tick, not just on the transition into working.

agentStatus is computed from the raw title and resumeCodexPermissionWaitForPty is invoked whenever it equals 'working', but this happens before prevStatus (pty.lastAgentStatus) is read a few lines below. Spinner-frame titles retrigger onTitle many times per second while an agent is working (per comments elsewhere in this file, "synthetic spinner ticks arrive ~12.5x/sec per working pane"), so this hook is invoked repeatedly for the entire duration of the working state instead of once on the working transition. Moving the check after prevStatus is known and gating on prevStatus !== 'working' would avoid the redundant repeated calls while still satisfying the "resume on next working hook" requirement from the PR description.

♻️ Proposed fix: gate on transition into working
     const agentStatus = detectAgentStatusFromTitle(rawTitle)
-    if (agentStatus === 'working') {
-      this.resumeCodexPermissionWaitForPty(ptyId)
-    }
     let ptyRecordChanged = false
     const pty = this.ptysById.get(ptyId)
     if (pty) {
       const prevStatus = pty.lastAgentStatus
       const prevTitle = pty.lastOscTitle
+      if (agentStatus === 'working' && prevStatus !== 'working') {
+        this.resumeCodexPermissionWaitForPty(ptyId)
+      }
       const observedAt = this.nextTitleObservationSequence()

Also applies to: 6279-6296

src/main/runtime/orca-runtime.test.ts (1)

6975-6986: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Test is well-isolated; consider relocating out of the unrelated describe block.

The new test doesn't exercise the "terminal side-effect fact channel" (no onTerminalSideEffects/batches usage) — it tests Codex permission-resume routing via a separate resumeCodexPermissionWait dependency. Consider moving it to its own describe block (or alongside other Codex-permission-resume tests) for discoverability, since this describe block is otherwise scoped to pty:sideEffect batch behavior.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b46fe951-2241-4107-8ed9-e27cc1b82788

📥 Commits

Reviewing files that changed from the base of the PR and between cfd26de and ca3d00c.

📒 Files selected for processing (6)
  • src/main/agent-hooks/server.codex-permission-resume.test.ts
  • src/main/agent-hooks/server.ts
  • src/main/index.ts
  • src/main/runtime/orca-runtime.test.ts
  • src/main/runtime/orca-runtime.ts
  • src/shared/agent-status-types.ts

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