Skip to content

fix(native-chat): start Windows chats without reading process creation times - #25718

Merged
brennanb2025 merged 10 commits into
mainfrom
brennanb2025/windows-agent-tree-job-object
Oct 6, 2026
Merged

brennanb2025 merged 10 commits into
mainfrom
brennanb2025/windows-agent-tree-job-object

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 32 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​722 $\color{#cf222e}{\Huge{\mathbf{−}}}$​657 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​65
Prod 52 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​316 $\color{#cf222e}{\Huge{\mathbf{−}}}$​538 $\color{#cf222e}{\Huge{\mathbf{−}}}$​222

ELI5

On Windows, native chat would sometimes refuse to start: Orca treated the chat as unsupported on that machine. It refused because Orca couldn't read the time each process was started. Orca only used those times as bookkeeping for stopping the agent later; the chat itself never needed them. Now Windows stops the agent the common way, by ending the process tree of the agent Orca launched and still holds, so chat no longer depends on those times and always starts.

What Changed

The problem. Orca reads Windows process start times through a native process-table add-on. Whenever that add-on couldn't report them (for example, it failed to load), native chat for Claude and Codex was refused outright. Structured chat checked for this in three places:

  • the "can this location run a chat" check for Claude and for Codex;
  • a second refusal when launching Codex;
  • startup itself, which threw if the agent's start time couldn't be read.

Start times were used only to stop the agent safely on Windows. Claude took a snapshot of the agent's child processes with their start times, so a later forced kill could never hit an unrelated process that had reused a process ID. After a restart, the same times decided whether a saved process ID still belonged to our agent.

Before → after for the user

  • Before: on a Windows machine where Orca can't read process start times, a Claude or Codex chat never opened. Orca refused it as unsupported on that machine, and depending on where it was started, fell back to a terminal agent instead. Settings said so too: "Windows falls back to it unless Orca can read process start times."
  • After: the chat starts, and the Settings line no longer mentions process start times. Stop, closing the chat and quitting Orca still end the agent.

Mechanism

  • Stop on Windows. After the usual graceful step, Orca runs taskkill /T /F on the agent process it launched and still holds. It reports the agent's processes as gone only when taskkill exits 0; otherwise it reports "couldn't confirm". This applies to both Claude and Codex.
    • Codex used to report success unconditionally on Windows; it now reports the real result.
  • After a restart on Windows. Orca never kills by a saved process ID. If a saved owner still looks alive, it is released without being touched.
  • Startup. A start time that can't be read is recorded as unknown instead of refusing the chat, on every platform. The restart path already handles an unknown owner: it releases it without sending any kill.
  • Normal close on Windows. When Claude exits by itself after its input ends, the close counts as done, with no tree kill.
  • Settings. The "Use updated structured native chat" scope line no longer says Windows falls back unless Orca can read process start times (all six languages).
  • Deleted:
    • the location checks and the Codex launch refusal;
    • Claude's Windows start-time snapshot and the verifier that only it used (windows-descendant-exit-verification.ts), plus its now-unused process-walk helper.
  • Renderer. It no longer waits for the "can read process start times" flag before it stops re-checking WSL availability.
    • Temporary difference: the host keeps publishing that flag because clients from before this change still wait for it. Follow-up: a small PR deletes windowsProcessStartTimeAvailable from the runtime status and the shared contract once those clients are out of support.

macOS and Linux behave as before, apart from the startup throw being gone.

Why

The common pattern stops the agent on Windows with a tree kill on the child process it still holds, and needs no start times. Because Orca still holds the child, the process ID can't belong to anyone else while Orca uses it, which is exactly the protection the start times were adding.

Differences from the common pattern, labelled:

  • Normal close on Windows — intended. Stop and close first end Claude's input and give it a moment to exit by itself, which an idle Claude does. When it does, Orca treats the close as done and doesn't verify that processes Claude started have ended. Background work still running shows as stopped, as before. Codex already closes this way on main, and so does the common pattern. Only when Claude doesn't exit in time does Orca run taskkill /T /F on it.
  • Orca crash — unchanged. If Orca itself is killed, Windows kills the agent process, but processes the agent started (shells, tool servers) can keep running. Node puts its direct children in a job that dies with Orca but lets their children leave it. This PR doesn't change that, and the common pattern doesn't cover it either.
    • Keeping the whole tree in a Windows job object would close that gap. It needs a native API and one extra process per agent, so it is proposed separately rather than built here.
  • Keeping a chat's claim alive — intended, all platforms. While a chat runs, Orca periodically renews its claim that this Orca owns it. Before, each renewal looked up the agent's process ID and start time; now, for an agent this Orca launched and still holds, holding it is the proof. Renewal stops as soon as Orca sees the agent exit, or when Orca itself exits, so a dead agent's claim still runs out. An agent this Orca doesn't hold is still checked the old way. This keeps renewals working where start times can't be read (otherwise a crash would date the chat's end to when it started), and it drops a process-table read every 10 seconds.
  • Alternatives considered:
    • Keep the start-time gate and only explain the refusal better. The user still can't chat.
    • Build the job-object design now. That is an Orca-only mechanism with a native rebuild, and it is pending a separate decision.

Linked Issue

N/A: maintainer change, from an internal report.

Visual Proof

Windows (high-spec test machine), isolated test profile, stand-in agents. To reproduce a machine that can't read process start times, the creation-time add-on was hidden in each build. The runtime status confirmed the flag was absent on both builds.

  • Before = main 9def4b9ba19. After = this branch at 6935e0de2da for the start-up screenshots, and at 1f17327cc28 for Stop.

Before: Claude and Codex chats both fail to start ("Chat could not be started." with Retry), and the sidebar shows the chat as failed.

Before: Claude chat could not be started
Before: Codex chat could not be started
Before: sidebar shows the chat failed

After: the Claude chat starts and answers. The Codex chat opens as a native chat, but its stand-in crashed on this machine, so no reply is shown.

After: Claude chat answers
After: Codex opens as a native chat

After, Settings: the scope line no longer mentions process start times.

After: Settings scope line

After, closing a chat with background work running: no "status unavailable" or any other new text appears.

After: background work before close
After: after close

Stop with a running tool (after = 1f17327cc28 with start times hidden; before = main with them available). Both read "Stopped.", with no notice or new status text, and look the same.

After: tool running
After: Stopped
Before (main): tool running
Before (main): Stopped

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Tests that now assert the new behaviour:

  • Windows chat support is answered "supported" whether or not start times are readable.
  • Codex launch resolves on such a host.
  • An unreadable start time is recorded as unknown.
  • Windows recovery releases a matched owner without signalling it.
  • Claude's Windows reap:
    • one taskkill on the held root;
    • "gone" only on exit 0;
    • never on a root that already exited;
    • never snapshots.
  • Codex teardown reports the taskkill result.
  • Lease renewal (darwin and win32): a held agent with no start time keeps renewing with no process lookup; renewal stops after its exit; after a crash the chat's end comes from the last renewal, not the start; an agent this Orca doesn't hold still uses the lookup.

Test runs:

  • vitest: the direct tests plus every test file that imports or mocks a changed module, 102 files.
    • 11 renderer TabBar/status-bar tests timed out under machine load and pass when run alone.
  • Typecheck (node, web, cli): no new errors. The remaining ones are on main already: stream-json, and one journal test type error.

Windows QA (stand-in agents; after = this branch, before = main):

  • Stop with a running tool: on both builds, the agent and the processes it started normally are gone within 1 second, and the chat reads "Stopped.".
  • Close a chat or quit Orca mid-tool: the agent is gone within 1 second.
  • Detached processes: a process the stand-in had started detached kept running after Stop, close and quit, on main too (Stop and close were run on main). This PR doesn't change that.

Not verified:

  • A Codex reply on Windows. The stand-in crashed; the Codex chat did open natively.
  • A real taskkill failure on Windows. Unit tests only.
  • Real Claude or Codex on Windows. Stand-ins only.
  • Re-running Windows QA after the last two main merges. Main moved Claude's close sequence into a shared close, and this PR's Windows close rules moved into it as an opt-in setting with the same unit tests. Windows QA ran on 1f17327cc28, before those merges.

Real Claude CLI suite: run once on the final head, 10/10 passed.

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • SSH/WSL: native chat stays local-only and non-WSL; those checks are unchanged.
  • Mobile and remote clients: the only shared field is still published, so older clients behave as before.
  • Performance: fewer process-table reads on Windows, because the snapshot is gone. Stop runs one taskkill.exe per stop, as before.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…n times

Native chat on Windows refused to start ("Orca can't run this agent in a chat
here") whenever the process-table addon could not report process creation
times. Chat never needed them; only the bookkeeping around stopping the agent
did.

Windows now follows the common pattern: Stop ends the agent's tree with
`taskkill /T /F` on the child Orca still holds, and reports the tree gone only
when taskkill exits 0. A saved pid is never signalled after a restart.

- Remove the Claude and Codex location gate and the Codex launch refusal.
- A start time that cannot be read is recorded as unknown instead of refusing
  the session; recovery already releases such an owner without signalling it.
- Delete Claude's Windows creation-time descendant snapshot and its verifier.
- Codex's Windows teardown reports the real taskkill outcome.
- The renderer no longer waits on the capability flag; the host keeps
  publishing it for older clients (temporary).
…oven close

On Windows an idle Claude leaves on its own once its stdin ends, so every Stop
and close read as unproven: live background work settled as unknown and the
trace logged a close that "did not finish cleanly". As with the Codex close,
that exit is now the close and Orca makes no claim about processes Claude
started; a forced close still rests on taskkill's own report.

Also drop the Settings clause about Windows needing process start times, and
fix comments and the tracked process-enumeration doc that still described the
removed start-time gate and descendant snapshot.
An owner recorded without a process start time could never renew: the renewer
re-proves every live owner by PID identity, and with no start time and no
spawn-token echo that probe is indeterminate. The lease's last renewal then
stayed at the spawn, so a turn cut by an Orca crash was dated to its own start,
and every tick logged a failed renewal and split the batch into one store
transaction per chat.

The runtime now renews a lease for a child it still holds at the record's
fence and whose exit it has not received, recorded as a `held-child` match.
Receipt of the exit ends that proof before the exit is settled, and a restart
holds no child, so a dead owner's lease still expires and restart adjudication
is unchanged. Records this runtime does not hold keep the PID probe.

Also correct the identity probe's comment about shipped addons and note that
recovery's stop ladder is POSIX-only.
…tried close

A Windows close counts Claude leaving on its own after its stdin ends as the
close. That shortcut also caught a retried close whose first attempt forced a
taskkill that failed: once the root exited, the retry returned true and the
failure read as a proven close. The tree reaper now records whether a reap
ever reached the live root, here, on an earlier close or from a transport
failure, and the shortcut applies only when none did; otherwise taskkill's
verdict stands and no new taskkill runs against the exited root.

Pin the platform on the existing tests that assume the POSIX close, and say
what `exit-proven` means on Windows in the acquisition-failure docs.
…wn handle

Lease renewal trusted a held child until the host settled its exit, and the
host hears of an exit late: Claude runs its close ladder and a store write
first, unexpected exits wait on one delivery chain shared by every chat, and a
Claude close that cannot prove its tree publishes nothing at all. A dead root
could keep renewing through that window, so a crash in it dated the cut turn
late. Renewal now asks the adapter, which owns the process handle and sees the
exit first: a child is held only while it is on record at the lease's fence
and its adapter still runs that exact acquisition with no root exit seen. An
adapter that cannot answer falls back to the PID probe. The stored
exit-received mark is gone.

The held-child read is now required by the runtime state and the renewer, and
a host-level test drives renewal through the real host wiring.
…agent-tree-job-object

# Conflicts:
#	src/renderer/src/i18n/locales/es.json
#	src/renderer/src/i18n/locales/ko.json
#	src/renderer/src/i18n/locales/zh.json
…agent-tree-job-object

Main moved the provider close ladder into the shared managed-process close
(#25204) and kept the Windows creation-time refusal in Codex launch resolution.
Resolved by keeping that refusal removed, taking main's pinned config variable,
and re-expressing this branch's Windows close rules on the shared close: the
Windows teardown reports taskkill's verdict ('exited' on exit 0, else
'unverifiable'), and a Claude root that leaves on its own after stdin end with
no forced reap on its tree is a proven close (Claude's close policy on win32).
…agent-tree-job-object

Main deleted the Claude and Codex location-support tests (#25791) that this
branch had edited; the deletion is kept. The Codex launch-resolution test still
pins that a Windows host without process creation times starts a chat.
@brennanb2025

brennanb2025 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

Review and QA summary

The problem. On Windows, native chat (Claude and Codex) refused to start whenever Orca couldn't read process start times, for example when the native add-on that reports them failed to load. Users saw "Chat could not be started." with a Retry that never helped. The chat itself never needed those times. Orca used them only as bookkeeping for stopping the agent.

What changes for the user

  • On such a machine, Claude and Codex chats now start.
  • The Settings line under "Use updated structured native chat" no longer says Windows falls back unless Orca can read process start times.
  • Stop, closing a chat and quitting Orca end the agent as before. Stop looks and behaves exactly as on main.

What the review fixed

  • Every Windows Stop marked background work as "status unavailable".
    • Cause: Claude usually exits by itself as soon as its input ends, before Orca's forced stop runs.
    • Fix: that exit now counts as a finished close, the same as Orca's Codex close on main and as the common pattern does. Orca makes no claim about processes Claude started (labelled in the body).
  • Retrying a close after a failed forced stop could report success. It now keeps the failure.
  • On hosts without start times, a running chat's ownership claim never renewed. A crash would then have dated the chat's end to when it started.
    • Fix: renewal now uses the agent this Orca launched and still holds, read from the process handle. This applies on every platform, and it also removes a process-table read every 10 seconds (labelled in the body).
  • Stale text: the Settings line in all six languages, comments that described the removed rule, and one tracked reference doc.
  • Merged main five times. Main moved Claude's close sequence into a shared close used by both providers, so this PR's Windows close rules now live there as an opt-in setting that only Claude on Windows turns on. A separate review confirmed the behaviour is unchanged by the move.

Deferred

  • P3: on a Windows host that can't read start times, if an agent's process ID is reused after a crash, that chat's interrupted turn stays "unverifiable" instead of "exited". It still resolves, and the user can keep chatting.
  • P3 (tests only): one exit-proof test would fail if the unit suite were run on Windows. CI doesn't run it there.
  • Nit: the WindowsTreeKiller type still says it returns nothing, though terminateWindowsProcessTree now reports whether taskkill succeeded. Callers ignore the result.
  • Unchanged, labelled:
    • If Orca crashes on Windows, processes the agent started can keep running.
    • A process an agent starts detached survives Stop, close and quit. QA showed main does the same.
  • Temporary: the host still publishes windowsProcessStartTimeAvailable for older clients. A follow-up removes it.

Verified

  • Windows test machine, stand-in agents, start times hidden. Before/after screenshots are in the body:
    • main refuses both chats;
    • this branch starts them, and the Claude chat answers;
    • the Settings line is updated;
    • closing with background work running shows no new text.
  • Stop with a running tool, on this branch and on main. Both read "Stopped." with no new notice. On both, the agent and the processes it started normally are gone within 1 second.
  • Close and quit mid-tool on this branch. The agent is gone within 1 second.
  • Tests:
    • unit tests for every changed path, run under both macOS and Windows platform settings;
    • every test file that imports a changed module;
    • typecheck;
    • lint;
    • the localization checks;
    • the real Claude CLI suite, once (10/10).
  • CI on edca20f715b (after a fifth main merge; each resolution was reviewed and changes no behaviour): every red check is main's own, none come from this PR.
    • static analysis (PR Checks) and Mobile Checks › verify both fail on mobile/src/session/use-mobile-native-chat-turn-disclosure.test.tsx being over the line limit (816 lines). That file comes from main's fix(native-chat): a long reply keeps its Working bar and Stop after a reload, from the host's turn #25751, and this PR changes nothing under mobile/. Other PRs fail the same way right now, e.g. runs 37514942851 and 37514469322.
    • The PR Checks verify aggregate fails only because of that.
    • That lint failure gates the rest of PR Checks, so on this head the test shards, packaging and cross-version jobs were skipped, not run. The headless server and LoC checks passed.
    • The test shards, packaging and cross-version jobs last ran on dba0ed45a8d (before the last two main merges), where everything was green.

Not verified

  • Windows QA after the last four main merges. It ran on 1f17327cc28; the moved close sequence is covered by unit tests.
  • A Codex reply on Windows. The stand-in crashed; the chat did open natively.
  • A real Windows taskkill failure. Unit tests only.
  • Real Claude or Codex on Windows. Stand-ins only.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 6, 2026 07:39
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 87184c25-ae94-4326-9dd2-248111cd395a
📥 Commits

Reviewing files that changed from the base of the PR and between dba0ed4 and a4a0b76.

📒 Files selected for processing (16)
  • src/main/codex/codex-structured-launch-resolution.ts
  • src/main/codex/codex-structured-session-state.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-claude-root-exit.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-provider-child.ts
  • src/main/runtime/structured-agent-session-failure-logging.test.ts
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json
💤 Files with no reviewable changes (2)
  • src/main/codex/codex-structured-session-state.ts
  • src/main/codex/codex-structured-launch-resolution.ts
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/zh.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/ko.json

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


📝 Walkthrough

Walkthrough

The changes revise Windows provider-process exit proof to use taskkill’s result or an eligible natural root exit, while retaining POSIX descendant-snapshot proof. Claude and Codex structured-chat support no longer depends on Windows process-start-time availability, and owner identities can have a null start time. The host lease renewer can use a matching, live child held by its runtime as identity evidence. Windows recovery no longer signals a saved owner PID. Runtime status and renderer capability data no longer expose the process-start-time availability field.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to a4a0b

No actionable merge-blocking regression is established by the reviewed evidence.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.06% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 51 functions across 51 files. (6 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely states the main change: Windows chats can start without readable process creation times.
Description check ✅ Passed The description covers the change, user impact, rationale, visual proof, testing, limitations, compatibility notes, and checklist. It identifies the linked-issue exception as a maintainer change and r…
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 47.06% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 51 functions across 51 files. (6 skipped: 6 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

…agent-tree-job-object

Main (#25159) kept the Windows creation-time refusal in Codex launch
resolution and moved the pinned config variable onto the Codex agent
definition; the refusal stays removed and main's pin is taken.

Main's host constructor grew to its line limit, so the held-child read moves
into the runtime state: it now takes the host's conversations (required) and
builds the renewer's read from them and the adapter.
…agent-tree-job-object

Main (#22302) gave the Codex launch resolver a launch-directory pin, changed
its test store fakes and added a test passing the creation-time seam this
branch removed. Kept main's store shape and new test without that seam, and
kept this branch's Windows test that resolves a launch without creation times.
@brennanb2025
brennanb2025 merged commit ab63890 into main Oct 6, 2026
31 of 34 checks passed
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.

1 participant