Skip to content

Fix premature Claude automation completion and inherited CI failures - #24878

Merged
nwparker merged 9 commits into
stablyai:mainfrom
AmoabaKelvin:fix/claude-wait-for-background-task-wakeups
Oct 6, 2026
Merged

nwparker merged 9 commits into
stablyai:mainfrom
AmoabaKelvin:fix/claude-wait-for-background-task-wakeups

Conversation

@AmoabaKelvin

@AmoabaKelvin AmoabaKelvin commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

ELI5

Claude can finish a background task before its follow-up message reaches the main agent. Orca now keeps the task cycle active until that message and its finishing turn complete, so an automation cannot close the terminal in the gap.

What Changed

The execution host tracks bounded task-notification debt and publishes an optional Claude task phase. Terminal waits check that phase against the current session, launch and host, using the existing per-pane status index. Retained ready text cannot finish the task cycle; fresh native rest can recover a lost finishing Stop. Duplicate notifications do not restart the cycle. Current child ends that beat their parent launch stay provisional in the same bounded map; a matching main launch confirms debt, while an earlier delivered notification stays settled.

Required CI also exposed two inherited regressions. The released-parser test now checks real parsing and bounded matches; file watcher reconciliation shares its original key list between the path index and purge. The purge reads fresh keys if a newer state reaches its functional updater, and retains all Windows case aliases.

Canceled local and remote turns keep their original idle boundary despite late tool hooks. Remote cancellation is reconciled to the execution owner with an optional, session/launch/turn-fenced command over the current SSH or WSL transport.

Why

Hook status alone did not prevent the actual automation observer from accepting Claude's native ready title in the owed-message gap. The narrow owner phase fixes that case while preserving ordinary Claude readiness. The execution host owns cancellation and expiry; readers do not invent remote process state. A missing notification expires after 60 idle seconds instead of leaving the pane working forever.

Linked Issue

Fixes #23942

Visual Proof

A real scrubbed Claude PTY capture and its metadata are committed as the automation oracle. Production HTTP/relay ingress and the actual runtime completion observer replay identical captured hook and ready-byte sequences before and after. No current-head rendered Orca screenshot, video, or hidden-app validation is claimed.

Testing

  • Automated regression tests added.
  • Same corrected production completion test fails on both parent main and the original PR, then passes with the repair.
  • An earlier supplementary default-config run passed 1,741 relevant tests with 10 platform/optional skips; the final integrated experimental reliability gate passes 148 tests using the repository Vitest configuration, with 40 retirement, child lifecycle and host expiry tests passing separately, including repeated messages both during and after finishing, shell duplicates, and blank/padded Agent/Bash/Monitor IDs and reordered child-end/launch/notification posts through both execution paths. Final node/web types, default lint, changed-code-quality and manifest checks pass. Transcript privacy scan passes.
  • The preexisting cross-version test on CI main expected the retained parser to be absent. Its test-only correction executes actual released dense parsing, verifies the range cap and rejects malformed input; synthetic missing-package refusal tests stay intact. Complete configured cross-version suite passes 119 tests in 21 files after cold caches materialize, without timeout changes.
  • The inherited watcher regression is repaired with 61 configured tests across five files. The original strict 2,001-key scan assertion remains intact; delayed functional updates and duplicate Windows aliases have new controls. Node/web types, format and changed quality pass.
  • Integration with main preserves all reviewed production files and eight author and repair commits, resolves only two test conflicts, and keeps both sparse and bounded released-parser controls. Affected configured suites pass 182 tests across 27 files; node/web types, the broader quality check of 367 files with zero findings, and validation of all 144 manifest gates pass. Fresh exact-head CI remains required.
  • Local macOS HTTP/runtime replay verified; physical SSH/WSL, native Windows/Linux and authenticated end-to-end Orca automation remain CI/manual coverage gaps.

Review

One shared host deadline timer, at most 256 tasks per pane, and zero full-store reads with 512 irrelevant agents on the indexed wait path. Optional wire fields are safe for mixed versions; older hosts retain their existing behavior. No new stream opcode, reader cache or remote timeout authority.

Agent skill upstream boundary

  • Not applicable; no upstream skill-installer material is copied.

Notes

A lost notification intentionally delays completion by up to 60 idle seconds plus the existing poll interval. Experimental gate documents platform and soak gaps. Final exact-head CI and bot findings must pass before merge.

Checklist

  • Explained the user-visible before/after, mechanism and reason for this approach.
  • Reviewed correctness, owner boundaries, performance and mixed-version behavior.
  • Cross-platform and SSH/WSL impact considered and coverage limits documented.
  • Current-head CI complete; merge stays held until it passes.

@AmoabaKelvin
AmoabaKelvin force-pushed the fix/claude-wait-for-background-task-wakeups branch 2 times, most recently from 51d378f to 4bee374 Compare October 2, 2026 20:42
@AmoabaKelvin
AmoabaKelvin marked this pull request as ready for review October 2, 2026 20:57
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Claude background-task launches and owed notifications now contribute to pane status. The hook server and relay schedule status updates when notification leases expire. Wake-up state also informs terminal readiness. The changes add guarded remote interrupt reconciliation across relay, SSH, and WSL connections, with tests and captured fixtures covering task lifecycles, cancellation, expiry, and readiness.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to 1679b

Hook events can arrive out of order. If a background agent's end event arrives before its launch is recorded, the pane can be marked finished while the task's notification is still pending. That reintroduces the premature-completion problem this PR is meant to fix. A test-coverage concern about tracking-cap eviction also remains open. Address both before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 1679b

The inspected paths preserve pane, connection and turn ownership while preventing premature completion. Remaining uncertainty concerns lost notifications, restart recovery and mixed-version operation; no introduced security vulnerability was established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected new reconciliation path is bounded to one pane on an existing connection, with launch, session and turn checks at the owner. Its operation reconciles cached Claude cancellation state rather than introducing an arbitrary-command or credential-access API.

Trust Boundaries and Controls

  • observed — The readiness veto requires a fresh pane-or-handle-associated Claude row, hook origin, session metadata, matching host connection and the current launch token when available. Restored-unconfirmed and provider-session-only rows are excluded from live-row selection. Transport forwarding separately rejects foreign connections and stale or disposed multiplexers.

Resilience and Maintainability Implications

  • observed — Cancellation preserves the lead's idle boundary without retiring independently running child work. Notification expiry remains owner-managed, and pane/session cleanup removes transient tracking; timer publication rechecks ownership rather than letting readers infer remote process state.

Hardening Proposals

  • proposed — If automation requires a stronger completion guarantee, expose degraded tracking at capacity and reconcile unobserved launches from authoritative owner inventory. This would distinguish bounded best-effort readiness from proof that no follow-up work remains.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.59% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 85 functions across 63 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #23942 requires Orca to avoid completing a terminal run while Claude still owes background-task notifications. The task-delivery tracker records agent and shell launches, marks owed notification…
Out of Scope Changes check ✅ Passed The changes support issue #23942 by tracking owed work and preventing premature terminal completion. Cancellation reconciliation, remote owner fencing, expiry cleanup, status plumbing, and their tests…
Title check ✅ Passed The title clearly identifies the primary change: preventing premature Claude automation completion. It also mentions inherited CI fixes, which are present but secondary.
Description check ✅ Passed The description covers the change, rationale, linked issue, testing, review considerations, and known coverage gaps. It also states that exact-head CI is pending. The visual-proof section explains the…
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.59% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 85 functions across 63 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@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 (1)
src/shared/claude-background-task-inventory.ts (1)

85-87: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Skip empty IDs after trimming.

A whitespace-only id is pushed as "". Nothing is tracked under an empty ID, so the empty entry is harmless today. It does break the documented contract that unlisted entries are counted but not listed.

Proposed fix
-      if (typeof obj.id === 'string') {
-        runningNonAgentTaskIds.push(obj.id.trim())
-      }
+      const id = typeof obj.id === 'string' ? obj.id.trim() : ''
+      if (id.length > 0) {
+        runningNonAgentTaskIds.push(id)
+      }

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 7db37d50-723b-4190-94f9-176c9518a999

📥 Commits

Reviewing files that changed from the base of the PR and between 44a2ee5 and 4bee37492040b18c60b032f7e0c28c9bda44f11b.

📒 Files selected for processing (28)
  • src/main/agent-hooks/server-claude-cancel-captures.test.ts
  • src/main/agent-hooks/server-claude-owed-notification-expiry.test.ts
  • src/main/agent-hooks/server-relayed-claude-cancel.test.ts
  • src/main/agent-hooks/server/server-cleanup.ts
  • src/main/agent-hooks/server/server-lifecycle.ts
  • src/main/agent-hooks/server/server-status-update.ts
  • src/main/agent-hooks/server/server-types.ts
  • src/main/ai-vault/session-scanner-claude-subagents.ts
  • src/relay/agent-hook-request.ts
  • src/relay/agent-hook-result-retry-scheduler.ts
  • src/relay/agent-hook-server-claude-owed-notification-expiry.test.ts
  • src/shared/__fixtures__/claude-task-notification-hooks.jsonl
  • src/shared/__fixtures__/claude-task-notification-hooks.meta.json
  • src/shared/agent-hook-listener-claude-task-notification.test.ts
  • src/shared/agent-hook-listener.ts
  • src/shared/agent-hook-listener/listener-event.ts
  • src/shared/agent-hook-listener/listener-state.ts
  • src/shared/agent-hook-listener/main-agent-turn-state.ts
  • src/shared/agent-hook-listener/providers/claude-events.ts
  • src/shared/agent-hook-listener/providers/claude-lifecycle-events.ts
  • src/shared/agent-hook-listener/providers/claude-pane-hold-evidence.ts
  • src/shared/agent-hook-listener/providers/claude-roster-state.ts
  • src/shared/agent-hook-listener/providers/claude-task-notification-delivery.ts
  • src/shared/agent-hook-relay.ts
  • src/shared/claude-background-task-inventory.ts
  • src/shared/claude-owed-notification-expiry-timers.ts
  • src/shared/claude-owed-task-notifications.ts
  • src/shared/claude-task-notification-text.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

@pullfrog pullfrog 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.

Important

The hold model is sound and the captured-hook tests are strong, but the shell-owe is gated on the literal 'Stop' turn event, so a shell first seen gone on a StopFailure turn is never marked owed and the row settles done before its pending notification — the same premature-done window this PR closes. I verified this with a scratch replay (details inline).

Reviewed changes

  • New hold model: claudeLaunchedBackgroundTasksByPaneKey records the main agent's launches and marks a task "owed" when it ends before its <task-notification>; owed work now counts as live work in claudePaneHoldEvidence, so the pane cannot read done in that window (claude-owed-task-notifications.ts, claude-task-notification-delivery.ts, claude-pane-hold-evidence.ts).
  • 60-second lease: ClaudeOwedNotificationExpiryTimers is armed on every stored row by the desktop hook server (commitStatusRowMutation override) and by the SSH relay (AgentHookResultRetryScheduler.armClaudeOwedNotificationExpiry), restating a settled row when an idle main agent was owed for a full lease (claude-owed-notification-expiry-timers.ts).
  • turnCompletedAt gate: a turn held only by an owed notification no longer stamps a completion, deferring the announcement to the wake-up turn (claude-events.ts).
  • Shared parsing and lifecycle wiring: readClaudeTaskNotification is shared with the AI-vault scanner; SessionStart/clear/SessionEnd/pane-move/reset all drop what is owed; claudeRunningNonAgentTask semantics widen to unlisted live work with no new wire field.
  • Tests: 46 new/updated tests replaying captured Claude 2.1.287 hooks plus hand-built lifecycle/lease cases. I ran the three new prod files (46 passed).

ℹ️ A relay or host restart can strand a held remote pane

Only the execution host holds claudeLaunchedBackgroundTasksByPaneKey and its lease. For a remote pane that is the relay, and the desktop arms nothing for remote rows: server-ingest-remote.ts mirrors only the claudeRunningNonAgentTask boolean into its sets, leaving its own launched map empty, so ClaudeOwedNotificationExpiryTimers.deadline() returns undefined and arm() clears. A relay restart during the owed window discards both the owed record and the timer; the desktop's row is persisted, remains working, and with no later row-producing hook nothing settles it — the remote-pane reaper explicitly requires connectionId === null (server-reaping.ts:33-46) and the connectionId-scoped clear has no production caller. The same shape applies to a desktop restart for a local pane whose agent process survived. The PR lists a live SSH host as unverified and covers "Claude exiting / terminal closing", but not a host restart. Worth confirming whether this is in scope or accepted; if accepted, a line in the Not-verified section would help.

Technical details
# host restart durability

## Affected sites
- `src/relay/agent-hook-request.ts:69` — lease armed only on a live stored hook
- `src/relay/agent-hook-server.ts:195-204` — `stop()` clears the listener state; `start()` restores nothing
- `src/main/agent-hooks/server/server-ingest-remote.ts:250-302` — remote ingress never populates the launched map or lead record, so the desktop arms no lease
- `src/main/agent-hooks/server/server-reaping.ts:33-46` — reaper excludes `connectionId !== null` panes

## Required outcome
- A pane held `working` solely by an owed notification must eventually settle even if the owning host restarts mid-window.

## Open questions for the human
- Is relay/host restart during the owed window in scope, or accepted because a later row-producing hook re-derives the row?

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@AmoabaKelvin

Copy link
Copy Markdown
Collaborator Author

Review follow-ups for 16d3ac2:

  • CodeRabbit, blank task id: addressed in 16d3ac2; a whitespace-only id is no longer recorded.
  • CodeRabbit, owed work cleared before exit ownership is checked: addressed in 16d3ac2. The listener no longer clears on a process-ending SessionEnd; the server clears only when it admits the exit from the pane's owner. Tests cover the owner's exit and a nested Claude's refused exit.
  • Pullfrog, relay or host restart during the wait: accepted, no change. What is owed is kept in memory, so after a relay or app restart the row is restored as unconfirmed working until the pane's next hook, the same as a row held by a background shell today.

@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 (1)
src/shared/agent-hook-listener-claude-task-notification.test.ts (1)

252-254: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert which task the cap evicts.

recordClaudeBackgroundTaskLaunch currently evicts a settled task. But if it evicts live a2 instead of announced a1, a257 can still be tracked and the final working assertion can still pass. The other cap test checks only map size and state. Assert that a1 is absent and a2 and a257 remain.

Suggested test assertions
     post({
       hook_event_name: 'Stop',
       background_tasks: [{ id: 'a257', type: 'subagent', status: 'running' }]
     })
 
+    const tasks = state.claudeLaunchedBackgroundTasksByPaneKey.get(PANE_KEY)
+    expect(tasks?.has('a1')).toBe(false)
+    expect(tasks?.has('a2')).toBe(true)
+    expect(tasks?.has('a257')).toBe(true)
+
     expect(post({ hook_event_name: 'SubagentStop', agent_id: 'a257' })?.payload.state).toBe(

ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e3e3d32d-17e0-47cb-85ab-95d2a866839d
📥 Commits

Reviewing files that changed from the base of the PR and between 4bee37492040b18c60b032f7e0c28c9bda44f11b and 16d3ac2655226a5ebdfda7a1b5c4925bf2e3d8c0.

📒 Files selected for processing (6)
  • src/main/agent-hooks/server-claude-owed-notification-expiry.test.ts
  • src/shared/agent-hook-listener-claude-task-notification.test.ts
  • src/shared/agent-hook-listener.ts
  • src/shared/agent-hook-listener/providers/claude-task-notification-delivery.ts
  • src/shared/claude-background-task-inventory.ts
  • src/shared/claude-owed-task-notifications.ts
💤 Files with no reviewable changes (1)
  • src/shared/agent-hook-listener.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@pullfrog pullfrog 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.

✅ No new issues found. The prior review's two findings are both addressed, and the SessionEnd refactor is sound.

Reviewed changes

  • StopFailure now owes a vanished shell: the shell-owe gate in claude-task-notification-delivery.ts widened from the literal 'Stop' to Stop/StopFailure, matching claude-events.ts's turn-boundary predicate. Adds the owes a vanished shell on a failed turn end too case.
  • Cap eviction no longer drops a running task: agent entries carry a settled flag; recordClaudeBackgroundTaskLaunch evicts only a settled entry and, when none exists, leaves the new launch untracked instead of displacing live work. oweClaudeAgentTaskNotification clears settled on a re-owed (resumed) agent. Covered by the past its cap never forgets a task that is still running case.
  • Empty task ids skipped: claude-background-task-inventory.ts no longer pushes "" for a whitespace-only id (the CodeRabbit nit).
  • SessionEnd drops owed state only for the owner: removed the eager map delete from normalizeHookPayload's SessionEnd branch. A nested Claude's exit no longer wipes the pane's owed map; owner gating is now solely transitionHookPresence, and the desktop clears via reconcileEndedProcessForPaneKeys → clearPaneState. New owner-exit and nested-exit tests added.

ℹ️ Relay does not clear its launched ledger on an owner exit

The removed eager delete used to also clean the relay's claudeLaunchedBackgroundTasksByPaneKey. The desktop still clears it (owner SessionEnd → reconcileEndedProcessForPaneKeys → clearPaneState), but the relay's applyEvent has no reconcile equivalent, so a remote pane's ledger entry and its unref'd lease timer survive an owner exit until pane teardown, surface retirement, or the next SessionStart. This is bounded and cannot republish a dead row — the stored row is ended/providerSessionOnly, and transitionHookPresence refuses an exit with no live owner, so the restate is dropped before forwarding — so it is not blocking. Worth noting only because the relay's owner-exit path is not exercised by a test today; the new relay expiry test covers the vanished-shell case.

Technical details
# relay launched-ledger retention after owner exit

## Affected sites
- `src/shared/agent-hook-listener.ts` SessionEnd branch — eager `claudeLaunchedBackgroundTasksByPaneKey.delete(paneKey)` removed in 16d3ac2
- `src/relay/agent-hook-server.ts:283-316` — `applyEvent` has no `reconcileEndedProcessForPaneKeys` equivalent; owner exit only sets `providerSessionOnly`
- `src/relay/agent-hook-request.ts:69` — lease armed for every stored row, including the SessionEnd row
- `src/shared/agent-hook-presence-transition.ts:9-31` — the guard that makes a restate for an ended owner a no-op

## Required outcome
- A remote pane held only by an owed notification must not leave stale ledger/timer state after the owning process exits; or the bounded retention must be an accepted tradeoff.

## Open questions for the human
- Is relay-side ledger retention until pane teardown/next session acceptable, or should the owner-exit path clear it on the relay too?

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@AmoabaKelvin
AmoabaKelvin force-pushed the fix/claude-wait-for-background-task-wakeups branch from 16d3ac2 to 9541c48 Compare October 3, 2026 11:23

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

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · Merge owed-task entries during pane authority transfers. · listener-state.ts:264-272

src/shared/agent-hook-listener/listener-state.ts:264-272
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Merge owed-task entries during pane authority transfers.

When a transfer moves a source task map onto a destination key that already has a task map, the generic helper replaces the destination map. Any destination-only owed task is lost. The pane-hold resolver reads owed-task state from this map, so the pane can settle before that task’s notification arrives if no other work holds it. Merge task entries at this task-specific move site.

Suggested fix
+function moveClaudeLaunchedBackgroundTaskEntries(
+  map: Map&lt;string, ClaudeLaunchedBackgroundTasks&gt;,
+  fromPaneKey: string,
+  toPaneKey: string
+): void {
+  for (const [key, sourceTasks] of Array.from(map.entries())) {
+    if (key !== fromPaneKey &amp;&amp; !key.startsWith(`${fromPaneKey}\0`)) continue
+    const destinationKey = `${toPaneKey}${key.slice(fromPaneKey.length)}`
+    const destinationTasks = map.get(destinationKey)
+    if (destinationTasks) {
+      for (const [taskId, task] of sourceTasks) {
+        destinationTasks.set(taskId, task)
+      }
+    } else {
+      map.set(destinationKey, sourceTasks)
+    }
+    map.delete(key)
+  }
+}
+
 export function movePaneCacheState(
   state: HookListenerState,
   fromPaneKey: string,
@@
-  movePaneScopedMapEntries(state.claudeLaunchedBackgroundTasksByPaneKey, fromPaneKey, toPaneKey)
+  moveClaudeLaunchedBackgroundTaskEntries(state.claudeLaunchedBackgroundTasksByPaneKey, fromPaneKey, toPaneKey)

ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f5d1ca48-2a21-4005-a9a9-da484202a4be
📥 Commits

Reviewing files that changed from the base of the PR and between 16d3ac2655226a5ebdfda7a1b5c4925bf2e3d8c0 and 9541c485b910c4308df27e3d79a7b15518f2469d.

📒 Files selected for processing (7)
  • src/main/agent-hooks/server/server-cleanup.ts
  • src/main/agent-hooks/server/server-lifecycle.ts
  • src/relay/agent-hook-request.ts
  • src/shared/agent-hook-listener.ts
  • src/shared/agent-hook-listener/listener-state.ts
  • src/shared/agent-hook-listener/pane-scoped-cache-entries.ts
  • src/shared/agent-hook-relay.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/shared/agent-hook-relay.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@nwparker
nwparker force-pushed the fix/claude-wait-for-background-task-wakeups branch from 9541c48 to 217eb64 Compare October 5, 2026 05:45

@nwparker nwparker 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.

Claude owed task notifications remain host-owned; shared expiry timer, bounded task inventory, cancellation/session cleanup and relay parity reviewed. 66 focused tests and node typecheck pass. Missing task wake-ups can delay idle/completion up to 60 seconds.

Required PR Checks verify passed on this exact commit: 217eb643cc1a5f09731d6b9b6f5da636136d9b84.

@nwparker nwparker 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.

The captured task hook sequences reproduce the premature sidebar completion, but the original PR does not yet fix the automation completion path on current main. With real Claude Code 2.1.291 ready OSC bytes fed through OrcaRuntimeService.onPtyData, and a production hook row still working while a wake-up is owed, the actual runtime automation observer reports completed within 100 ms.

Additional production-ingress regressions reproduce an indefinite hold after Ctrl+C followed by a late tool hook on local and relay panes, and duplicate child-end delivery reopening already settled notification debt. Repairs and independent before/after regressions are being added before final approval or merge.

@nwparker
nwparker force-pushed the fix/claude-wait-for-background-task-wakeups branch from 217eb64 to cf051e0 Compare October 6, 2026 04:35
@nwparker

nwparker commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

The original status gap is reproduced through production HTTP/relay ingress. The same corrected captured-native-ready test against the real automation observer also reproduces premature completion on both parent main and the original PR; the repaired head is cf051e0e4f12595f64b0b176b338c9d8a12a747f.

This head adds the narrow owner task phase, host-authoritative cancellation reconciliation, duplicate-notification controls, and indexed runtime status reads. The final integrated named gate passes 118 tests; node/web typechecks, default lint, the actual changed-code-quality gate (65 files, zero findings), reliability manifest and transcript privacy checks pass. The relevant broader suite passed 1,741 tests with 10 platform/optional skips before the final clean main integration. No current-head hidden-app or physical SSH/WSL/native Windows/Linux proof is claimed.

The missing-notification fallback intentionally delays completion up to 60 idle seconds plus the existing poll interval; long foreground finishing work is held until its Stop or fresh native rest. Existing task and timer bounds are verified, and the actual indexed wait performs zero full-store reads with 512 irrelevant rows.

The PR body now describes the final implementation and coverage limits. Merge remains held for fresh exact-head CI and current bot findings. @coderabbitai review

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 04a978b6-18ea-49f1-ba09-491f8a7b2403
📥 Commits

Reviewing files that changed from the base of the PR and between 9541c485b910c4308df27e3d79a7b15518f2469d and cf051e0e4f12595f64b0b176b338c9d8a12a747f.

📒 Files selected for processing (57)
  • config/reliability-gates.jsonc
  • src/main/agent-hooks/server-claude-task-wakeup-lifecycle.test.ts
  • src/main/agent-hooks/server/server-cancel-verdict-latch.ts
  • src/main/agent-hooks/server/server-claude-status-rules.ts
  • src/main/agent-hooks/server/server-cleanup.ts
  • src/main/agent-hooks/server/server-constants.ts
  • src/main/agent-hooks/server/server-ingest-remote.ts
  • src/main/agent-hooks/server/server-lifecycle.ts
  • src/main/agent-hooks/server/server-persistence.ts
  • src/main/agent-hooks/server/server-remote-envelope-normalization.ts
  • src/main/agent-hooks/server/server-row-child-work-fold.ts
  • src/main/agent-hooks/server/server-status-identity.ts
  • src/main/agent-hooks/server/server-status-inference.ts
  • src/main/agent-hooks/server/server-status-update.ts
  • src/main/agent-hooks/server/server-types.ts
  • src/main/agent-hooks/wsl-hook-relay-deps.ts
  • src/main/agent-hooks/wsl-hook-relay-link.ts
  • src/main/agent-hooks/wsl-hook-relay-manager.test.ts
  • src/main/agent-hooks/wsl-hook-relay-manager.ts
  • src/main/orcad/orcad-entry.ts
  • src/main/runtime/__fixtures__/claude-ready-task-wakeup.meta.json
  • src/main/runtime/__fixtures__/claude-ready-task-wakeup.txt
  • src/main/runtime/claude-task-wakeup-indexed-read.test.ts
  • src/main/runtime/orca-runtime-resolve-authoritative-terminal-wait-permission.ts
  • src/main/runtime/orca-runtime-state-fields.ts
  • src/main/runtime/orchestration/worker-provider-session.ts
  • src/main/runtime/tui-idle-claude-task-wakeup.test.ts
  • src/main/runtime/tui-idle-hook-lane.test.ts
  • src/main/runtime/tui-idle-hook-lane.ts
  • src/main/ssh/ssh-agent-hook-interrupt-reconciliation.test.ts
  • src/main/ssh/ssh-agent-hook-interrupt-reconciliation.ts
  • src/main/ssh/ssh-relay-session.ts
  • src/main/startup/main-process-runtime-service.ts
  • src/relay/agent-hook-envelope-build.ts
  • src/relay/agent-hook-event-admission.ts
  • src/relay/agent-hook-interrupt-reconciliation.test.ts
  • src/relay/agent-hook-interrupt-reconciliation.ts
  • src/relay/agent-hook-request.ts
  • src/relay/agent-hook-server.ts
  • src/relay/relay-agent-hook-runtime.ts
  • src/relay/wsl-agent-hook-relay.ts
  • src/shared/agent-hook-cancel-verdict-latch.ts
  • src/shared/agent-hook-interrupt-reconciliation.ts
  • src/shared/agent-hook-listener.ts
  • src/shared/agent-hook-listener/listener-event.ts
  • src/shared/agent-hook-listener/main-agent-turn-state.ts
  • src/shared/agent-hook-listener/providers/claude-events.ts
  • src/shared/agent-hook-listener/providers/claude-lifecycle-events.ts
  • src/shared/agent-hook-listener/providers/claude-pane-hold-evidence.ts
  • src/shared/agent-hook-listener/providers/claude-roster-state.ts
  • src/shared/agent-hook-listener/providers/claude-status-build.ts
  • src/shared/agent-hook-listener/providers/claude-task-notification-delivery.ts
  • src/shared/agent-hook-relay.ts
  • src/shared/agent-hook-row-child-work-fold.ts
  • src/shared/agent-status-types.ts
  • src/shared/claude-owed-notification-resource-contract.test.ts
  • src/shared/claude-owed-task-notifications.ts
💤 Files with no reviewable changes (1)
  • src/main/agent-hooks/server/server-constants.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

AmoabaKelvin and others added 5 commits October 5, 2026 21:55
…s arrive

Claude wakes the main agent for each background task that ends, after the
task stops running. "Nothing running" was read as done, so automations
closed the terminal before the final turn.

Fixes stablyai#23942
…cap and nested exits

A failed turn's task list now marks a vanished shell owed like a normal turn end does. Past
the cap only an already-announced sub-agent is forgotten. A process exit clears what is owed
only where the server admits it from the pane's owner.
…istener-state

listener-state.ts passed the 300-line limit once main's and this branch's additions met.
@nwparker
nwparker force-pushed the fix/claude-wait-for-background-task-wakeups branch from 6f7e807 to 1679b0c Compare October 6, 2026 05:01

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 00e43d46-69ec-45fa-adea-977b2b2d012e
📥 Commits

Reviewing files that changed from the base of the PR and between cf051e0e4f12595f64b0b176b338c9d8a12a747f and 1679b0c.

📒 Files selected for processing (4)
  • config/reliability-gates.jsonc
  • src/main/agent-hooks/server-claude-task-wakeup-lifecycle.test.ts
  • src/main/agent-hooks/server/server-status-inference.ts
  • src/shared/claude-owed-task-notifications.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

@nwparker Thanks for the guidance. I’ll trigger a review of the current PR changes, with focus on task-wakeup completion, host-authoritative cancellation, duplicate notifications, and indexed status reads.

The reported local results do not establish exact-head CI or physical SSH/WSL and native Windows/Linux coverage. This review request does not change the merge hold.


✏️ Learnings added
Learnt from: nwparker
URL: https://github.com/stablyai/orca/pull/24878

Timestamp: 2026-10-06T05:16:05.294Z
Learning: For Claude task-wakeup handling in stablyai/orca, missing task notifications intentionally expire after 60 idle seconds, so terminal completion can be delayed by that interval plus the existing poll interval. Foreground finishing work remains active until its Stop or fresh native rest. Cancellation reconciliation is authoritative on the execution host. Runtime waits use the per-pane status index rather than full-store snapshots. Relevant modules include src/shared/claude-owed-task-notifications.ts, src/main/runtime/tui-idle-hook-lane.ts, and src/relay/agent-hook-interrupt-reconciliation.ts.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nwparker nwparker changed the title fix(agent-status): keep a Claude pane working until owed task wake-ups arrive Fix premature Claude automation completion and inherited CI failures Oct 6, 2026

@nwparker nwparker 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.

Independently reproduced premature completion using real captured Claude hook events and ready-title bytes through actual desktop/relay HTTP ingress, the runtime PTY path, and the automation completion observer. The original PR still completed too early. The repaired owner-produced notification/finishing-turn phase now holds completion until the finishing turn ends, including duplicate notifications before Stop for both agents and shells.

Reviewed local and execution-host-owned SSH/WSL cancellation, exact session/launch/turn fences, permission restoration, duplicate task ends, resumed task IDs, missing notifications, lost final Stop, snapshot stripping, and indexed readiness reads. A fresh bot finding on blank/padded launch IDs is fixed with canonical normalization; twelve desktop/relay production-ingress cases reproduce phantom debt or unmatched notifications before the fix and pass afterward. The actual runtime regression with 512 unrelated agents proves zero full status snapshot reads, including empty indexed results. Task records are bounded and timers are coalesced and unref'd; no recurring polling or new status cache was added.

Also reproduced a child ending before its parent launch event: both actual desktop and relay completion observers released too early. The existing bounded task map now preserves provisional end/receipt evidence until an own-main launch confirms ownership. End-before-launch, notification-before-launch, and notification-before-end cases hold through the finishing turn and complete after its Stop without phantom debt. Unknown, restored, wrong-session, and unanchored child evidence cannot mint an obligation; nested children remain provisional. Duplicate ends keep their first clock and real resumed tasks rearm correctly.

A fresh compatibility check exposed an identical pre-existing failure on its exact main baseline: a test expected @streamparser/json to be missing although main already retains its pinned development dependency. The test-only correction executes the actual released dense parser and verifies parsed output, match bounds, and malformed-input rejection. Separate synthetic missing-package refusal tests remain intact; no dependency, production parser, or workflow was changed.

Full unit CI also reproduced main's duplicate directory-cache key enumeration. The existing alias index and subtree purge now reuse the same complete key list only when the React updater still owns that exact cache object; otherwise purge enumerates fresh state. Alias detection remains intact, including Windows case-duplicate keys. The original 2,001-visit budget is unchanged and passes; 61 watcher regressions, node/web typechecks, formatting and changed quality pass. All other prior-head CI checks passed, including physical Windows/macOS SSH hosts and Docker SSH watcher journeys. Fresh full CI on this repaired head is required and has passed before this approval.

Integrated current main after two test conflicts: the original cache-enumeration budget remains strict, and released-parser assertions combine the upstream sparse-input check with bounded matches, path/line and malformed-input controls. All eight author/repair commits and 68 core reviewed files remain intact; all 143 upstream reliability gates survive alongside the new gate. The integrated tree passes 182 affected tests, node/web types, manifest validation and changed quality.

The disclosed low-risk limits are the 60-second idle grace for a genuinely missing notification and the 256-record per-pane tracking bound. Live SSH and native Windows/WSL were not exercised locally; relay/routing/owner tests and fresh platform CI cover the changed paths. The exact configured reliability gate passes 148 tests, plus 40 existing lifecycle/retirement/expiry regressions; typechecks, full lint, and the actual changed-code quality gate pass. Fresh full PR Checks CI on this exact head is green and all review threads are resolved. The repaired behavior satisfies the conditional merge request.

@nwparker
nwparker merged commit 634c437 into stablyai:main Oct 6, 2026
58 checks passed
Fartown pushed a commit to Fartown/orca that referenced this pull request Oct 6, 2026
…e the row after the activity window

Upstream stablyai#24878 added a relay case where a SessionStart for another
Claude session supersedes the row and must void a pending interrupt.
The fork's session guard drops a competing session that reports while
the owner was active in the last 30 seconds and the sender is not the
owner's process, so the row never changed and the interrupt still
applied. Production behaviour is the guard's intent; the case now moves
the clock past the activity window first, the same way the fork's own
relay ownership test admits a genuine replacement.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EjcinZAhBx3gqNYt9urQE2
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.

[Bug]: Automation terminal is retired ~60s after the agent idles, killing its pending background sub-agents

2 participants