[LivXue]

Summary

Two gaps in how Raven stops its cli sub-agents and what environment it starts them with.

Stopping a cli sub-agent on Windows. A cli sub-agent's timeout, cancel and stream-failure paths, and the kimi commands Raven runs to explain a failed Kimi Code connect, ended the process group with os.killpg, which Windows does not have. The AttributeError replaced the timeout error or the cancellation, and the CLI was left running. Where killpg is missing, the tree is now ended from the launcher's pid with taskkill /T /F while the launcher is still alive, and the launcher is then killed itself, the way raven/ops/transport.py already ends a command on Windows. Once the launcher has exited its pid may name another process, so nothing is killed by number. POSIX keeps the group kill.

A web key rotated through settings.set. settings.set rewrites ~/.raven/env when a web credential changes, but a POSIX cli or acp sub-agent reads that file only through the login-shell capture, which a running Raven takes once and keeps. Every later spawn started with the key the write had replaced, until a Connect, a Test, "Check again" or a restart took the capture again. A mirrored key's write now takes the capture again before the reply. Captures also run one at a time: a refresh that read the shell before the write, a Connect's for instance, could otherwise land after the write's own and put the replaced key back, and readers that arrive while the first capture runs now wait for it rather than each running the shell.

Windows is unchanged for keys. Nothing sources ~/.raven/env there, and a Windows sub-agent's environment is Raven's own with the registry's PATH, so a key held only in config does not reach it; #912 states this in the code. On POSIX the keys reach a sub-agent only through a source line in the user's shell startup files, which the onboard wizard adds only with the user's agreement, and a Windows delivery path would need a consent step of its own, so this PR does not add one.

Not changed here: an acp sub-agent that is already connected keeps the environment it was launched with until its next launch; a key written by raven onboard, which runs as its own process, reaches a running Raven's sub-agents at the next Connect, Test, "Check again" or restart; and on Windows the wizard still prints the POSIX source line when $SHELL is not bash or zsh.

Type

• Fix
• Feature
• Docs
• CI / tooling
• Refactor
• Other

Verification

Linux, Python 3.12, rebased onto main 182daca:

• uv run --frozen --python 3.12 --all-extras pytest -q with the proxy variables unset: 3 failed, 29667 passed, 158 skipped. The same 3 fail on main on this machine (an npm on PATH, a root-only permission check, a LibreOffice conversion); 0 introduced.

• uv run --frozen --python 3.12 --all-extras pytest tests/test_subagent_third_party.py tests/test_subagent_kimi_code.py tests/test_subagent_acp.py tests/test_subagent_host_env.py tests/test_rpc_console.py tests/test_rpc_settings.py tests/test_rpc_subagents.py -q: 1080 passed.

• Windows stop: tests/_windows_kill.py leaves a Windows host as it is, where the real taskkill runs, and elsewhere removes os.killpg and stands in for taskkill by ending the process group it is given, the launcher's tree here; either way it records each call. The launchers, their workers and the Kimi stand-in are Python started on sys._base_executable, because a Windows venv's python.exe is a redirector that runs the base interpreter as its child and returns a pid the script never sees. The tests have not run on a Windows host, and no CI job runs these test files there.

• Windows stop, measured here: the six tests that use the helper pass. With os.killpg removed for the whole session and a taskkill on PATH that ends the group it is given, they pass again through that taskkill (3 calls), and the POSIX-only Kimi test beside them is skipped. With sys.executable replaced by a wrapper that runs Python as its child, as a venv's python.exe does on Windows, both modes stay green; started on that wrapper instead, the two cancel tests fail on the pid they compare.

• Windows stop, against the fix: the six fail with AttributeError on main. Removing the alive check in the cli backend, the taskkill call or the kill after it in either module, or the cli backend's handling of a taskkill that times out, turns at least one of them red. Removing the POSIX group kill from the Kimi Code commands turns only the new test whose stand-in kimi starts a process of its own red (1 failed, 347 passed over the two test files).

• Rotated key: test_a_rotated_key_reaches_the_capture_the_next_spawn_reads saves a key twice through settings.set and reads the capture after each save, through a real bash that sources the mirror; on main the second read still returns the first key. Removing the refresh turns 12 tests red; refreshing before the mirror is rewritten turns that test red. Two existing tests that write a web key now stub the refresh, so they do not run the host's login shell.

• Concurrency: three tests force the interleavings with events: a refresh that read the shell first and lands after a later one, a second reader that arrives while the first capture runs, and readers waiting on a first capture that fails. Each failed in 3 of 3 runs on main and passes with the lock; removing the lock from either capture path, or either re-check inside the first one, turns one of them red.

• With --cov=raven --cov=raven_everos --cov-branch added to the full run, python scripts/coverage_gate.py diff --base-ref origin/main --threshold 90: 93.22% of the changed lines (55 of 59). The four left are except clauses for a process group or a process that is already gone.

• ruff check, ruff format --check, lint-imports, ty check and deptry over the Makefile targets, commitlint, the three repository gate scripts and pre-commit run --from-ref origin/main --to-ref HEAD: passed.

• git merge-file of raven/rpc/methods/console.py with each open PR that touches it (feat(plugins): add the everos-cloud-memory plugin and its host wiring #883, fix(ui-web): attach a picture dragged into the composer by its own path #900, fix: apply the configured timezone when creating cron jobs #911, fix: bound Windows environment expansion and preserve literal percent signs #912, fix: preserve active config aliases when updating settings #923, fix(rpc): treat a null section as absent when settings.set writes a key #925), and of raven/agent/subagent/backends/env.py with fix: bound Windows environment expansion and preserve literal percent signs #912 in both orders: no conflicts.

• Relevant tests pass locally

• Relevant lint / type checks pass locally

• User-facing docs or screenshots are updated when needed

No user-facing docs describe either behavior.

Risk

• Security impact considered
• Backward compatibility considered
• Rollback path is clear for risky changes

Saving a web key now waits for the user's login shell to start before the reply, as Connect and Test already do; the capture's 15 s timeout bounds it, and it can also wait for a capture already in flight. A capture that fails keeps the previous one and does not fail the save. No credential reaches a sub-agent that did not before; it is the same shell reading the same file, read again. On Windows a stop now waits up to 5 s for taskkill, and a worker that a launcher started and left behind after exiting outlives the stop there, since Windows has no group to address and an exited launcher's pid is not trusted. If taskkill is missing, fails or does not answer within those 5 s, only the launcher is killed, and a cli stop then waits for any worker still holding the launcher's output pipes to exit. Revert this PR to roll back.

Related Issues

#908, #912

[gloryfromca]

No blockers; this can merge as far as I am concerned.

I reviewed the effective PR delta from base 182dacac through this head, and also read git diff github/main...HEAD to account for the locally advanced target ref. I covered the Windows timeout/cancel callers, the existing taskkill precedent and PID-reuse boundary, login-environment capture and concurrent refresh ordering, settings mirror callers, relevant history, backward compatibility and test strength. I also checked AGENTS.md/CLAUDE.md, the applicable CONTEXT-MAP.md/CONTEXT.md terminology and layer constraints; I found no rules or architecture violations.

Verification: uv run --python 3.12 --extra dev pytest tests/test_subagent_third_party.py tests/test_subagent_kimi_code.py tests/test_rpc_console.py tests/test_rpc_settings.py -q passed (615 tests); Ruff check and format check passed on all nine changed files; git diff --check passed. The first default test environment could not collect because raven_everos was absent, and --all-extras could not build datrie because this runner has no compiler; the narrower documented dev extra supplied the test dependencies. At posting time, all completed GitHub checks pass, with two unit shards still pending.

[LivXue]

Windows compatibility review: the production cleanup passed the exercised native scenarios; one new test portability defect was reproduced.

Scope: the complete 9-file diff at head 4bf194d8bc7cbc2c9191fb7f6ff24d404bf60174, against base 182dacac001e6697b3260da4400d2f9e36ba536c. Checked CLI timeout/cancel/stream-failure callers, Kimi diagnostic cleanup, taskkill argv/PID lifetime, serialized environment capture, settings.set mirror ordering and all changed tests.

P2 - Make the Windows test helper callable on Windows. tests/_windows_kill.py:26 calls monkeypatch.delattr(os, "killpg") with the default raising=True. Since native Windows already lacks that attribute, the new CLI and Kimi cancellation tests both fail with AttributeError: killpg before exercising cleanup. Code inspection shows all five new users enter this helper first. Use raising=False or a presence guard, and provide native subprocess fixtures or explicitly guard the POSIX-only shell/signal fixtures. This helper is absent from the PR base.

Verification: native Windows / Python 3.13.12, isolated user/Raven/temp directories. Git HTTPS fetching failed, so I reconstructed the fixed-head snapshot from the exact base plus GitHub blobs. Its complete Git tree equals a29a3f8868b7ee74cdee9db91bdf6149c988b9d3, and all 4,458 snapshot blobs match that tree. The complete local tree diff and its whitespace check passed.

The reused uv environment has matching pyproject.toml/uv.lock. PYTHONUTF8=1, UV_NO_SYNC=1; PYTHONPATH selects the tested snapshot and plugin roots, and the imported Raven source was asserted. $R = 'C:/Users/livxu/AppData/Local/Temp/raven-windows-review/3a29a2480c9547a4a357f8f9b019ac13'; UV_PROJECT_ENVIRONMENT=C:/Users/livxu/AppData/Local/Temp/raven-windows-review/7083af8abaec45c385fd83020f19e718/venv.

uv run pytest tests/test_subagent_third_party.py::test_cli_backend_cancel_without_killpg_ends_the_tree_through_taskkill -n 0 -x --tb=short --basetemp "$R/checks/cli-test/pytest" --junitxml "$R/checks/cli-test/junit.xml"
# 1 failed at the new helper's line 26.
uv run pytest tests/test_subagent_kimi_code.py::test_a_connect_that_goes_away_mid_ask_ends_its_kimi_through_taskkill_without_killpg -n 0 -x --tb=short --basetemp "$R/checks/kimi-test/pytest" --junitxml "$R/checks/kimi-test/junit.xml"
# 1 failed at the same line.
uv run pytest tests/test_subagent_third_party.py -k 'login_shell_env or refresh_that_read or readers_that_find_no_capture' -n 0 -x --tb=short --basetemp "$R/checks/capture/pytest" --junitxml "$R/checks/capture/junit.xml"
# 15 passed.
uv run pytest tests/test_rpc_console.py tests/test_rpc_settings.py::test_secret_key_writes_string -k 'mirror or mirrored or capture or rotated_key or secret_key_writes_string' -n 0 -x --tb=short --basetemp "$R/checks/settings/pytest" --junitxml "$R/checks/settings/junit.xml"
# 26 passed, 1 skipped (real POSIX shell-mirror case).
uv run pytest tests/test_subagent_host_env.py -k 'capture or registry or refresh' -n 0 -x --tb=short --basetemp "$R/checks/host-env/pytest" --junitxml "$R/checks/host-env/junit.xml"
# 5 passed, 1 failed: extensionless executable fixture reported missing.
uv run pytest C:/Users/livxu/AppData/Local/Temp/raven-windows-review/3a29a2480c9547a4a357f8f9b019ac13/test_native_cleanup.py -s -n 0 -x --tb=short --basetemp "$R/checks/native/pytest" --junitxml "$R/checks/native/junit.xml"
# 5 passed: real taskkill, both launcher and worker exited in each case.
# Exact-base snapshot, with PYTHONPATH repinned to $R/base:
uv run pytest C:/Users/livxu/AppData/Local/Temp/raven-windows-review/3a29a2480c9547a4a357f8f9b019ac13/test_native_cleanup.py -k cli-timeout -s -n 0 -x --tb=short --basetemp "$R/checks/base-native-base/pytest" --junitxml "$R/checks/base-native-base/junit.xml"
# 1 failed as expected: the production timeout path calls missing os.killpg.
uv run pytest tests/test_subagent_host_env.py::test_windows_probe_finds_agents_on_the_captured_path -n 0 -x --tb=short --basetemp "$R/checks/host-control-base/pytest" --junitxml "$R/checks/host-control-base/junit.xml"
# Same executable-fixture assertion failed on the exact base; inherited.

The scratch native probe covers CLI timeout, cancellation and output-sink failure, plus Kimi-helper timeout and cancellation. It uses owned Python launcher/worker processes under a path containing spaces and non-ASCII characters, real Windows process handles, and unmocked taskkill; it does not launch the actual Kimi vendor CLI. The base control confirms the CLI runtime fix changes the observed failure.

No full repository suite, real vendor authentication or POSIX login-shell integration ran here. The inherited executable-fixture failure and documented already-exited-launcher limitation are not treated as new findings. Windows credential delivery remains unchanged as described by the PR.

[gloryfromca]

Blocking: the new Windows cleanup tests must run on native Windows instead of failing in their shared helper.

This report holds. I reproduced the exact AttributeError: killpg by first removing os.killpg and then calling windows_kill_path; its default raising=True fails immediately. All five new callers enter that helper, and the affected CLI/Kimi fixtures also rely on POSIX sh/sleep behavior, so changing only raising=False would not by itself establish native Windows coverage.

The two focused cleanup cases still pass on Linux (2 passed in 4.35s), and the native scratch results support the production fix. My stance changes solely because the committed Windows-facing regression tests fail on the platform they are meant to exercise. The helper and callers need to become portable while retaining real cleanup assertions.

[LivXue]

Fixed in 95fbf84: the Windows stop tests now run on a Windows host instead of failing in their shared helper, and they still assert real cleanup.

Reproduced first at 4bf194d, with os.killpg removed for the whole session and a taskkill on PATH, which is how a Windows host looks to these tests: all five tests that call the helper failed with AttributeError: killpg at tests/_windows_kill.py:26, and taskkill was never reached.

What changed, in tests only:

• Where os.killpg is already missing, windows_kill_path replaces nothing. It finds the real taskkill (skipping if there is none), lets it run and records each call. Where killpg exists, it removes it and stands in for taskkill by ending the process group it is asked about, which is the launcher's tree here, so both hosts assert the same calls and the same deaths. A call is now recorded without the program path, since the path shutil.which finds differs by host.
• The sh/sleep fixtures are Python: a launcher that starts one worker on its inherited pipes and notes both pids (it stays, or exits once noted), a sleeper, and a Kimi stand-in found through resolve_program the way the ask finds the real one. Liveness is read with raven.utils.pid.pid_alive, since os.kill(pid, 0) is no probe on Windows. The cancel test now also asserts that the launcher's worker is gone, which the record-only helper could not.
• Neither report named this one, and it would have failed the two cancel tests on a Windows host even with the helper fixed: a venv's python.exe there is a redirector that runs the base interpreter as its child (bpo-34977), so the pid a spawn of it returns is not the pid the script reports. The scripts are started on sys._base_executable, the interpreter multiprocessing launches from a venv for the same reason (bpo-35797).

This deviates from raising=False plus POSIX-only guards: the fixtures are native instead, so all six, the five you ran and the one 2e7cffe adds, keep running on Windows. Only test_an_ask_that_runs_out_takes_what_its_kimi_started stays POSIX-only, since it asserts a process group.

Also in this push: 2e7cffe adds the two tests the diff coverage gate asked for, a taskkill that does not answer and readers waiting on a capture that fails.

Verification at 95fbf84, Linux, Python 3.12:

• The six tests and that POSIX one: 7 passed. With os.killpg removed for the session and that taskkill on PATH: 6 passed, 1 skipped, and the taskkill ran 3 times. With sys.executable replaced by a wrapper that runs Python as its child and waits, the shape of a Windows venv: green in both modes. With the scripts started on that wrapper instead: the two cancel tests fail on the pid they compare, in both modes.
• The eight mutants of the stop paths in cli_agent.py and kimi_code.py each turn at least one test red.
• Full suite: 3 failed, 29667 passed, 158 skipped. The 3 failures also fail on main on this machine. Diff coverage: 93.22% of the changed lines (55 of 59).

Not run: a Windows host. A re-run of the native probe on this head would close that.

Found on the way, and stated in the description rather than changed: when taskkill is missing, fails or does not answer within its 5 s, only the launcher is killed, and a cli stop then waits for any worker still holding the launcher's output pipes to exit.

Untouched: the tests/test_subagent_host_env.py executable-fixture failure, which the review measured on the exact base.

[gloryfromca]

No blockers; this can merge as far as I am concerned.

The prior Windows-test blocker is fixed. I reviewed the complete 4bf194d8..95fbf84f delta, including both new test commits, the shared helper, all six Windows-branch callers, PID identity and liveness handling, timeout fallback, the already-exited-launcher case, concurrent capture failure, relevant repository rules, and whether the assertions were weakened. The fixtures are now native Python processes, preserve real launcher/worker death assertions, and run real taskkill when os.killpg is absent; the POSIX stand-in exercises the same production branch without skipping it. I found no new defect.

Exact-head verification: the eight changed-path tests passed; the two affected suites passed (348 passed); Ruff check, Ruff format check, and git diff --check passed. The immutable PR diff is base 182dacac001e to head 95fbf84fe325 with SHA-256 fa76fced03ebd70d7d2ba772f97988cc6707de62bad2c763c306eb3314de005d. Risk assessment recommends merge with human review: moderate impact if wrong, low regression likelihood, partial regression protection because this corrected head has not yet run its unit tests on native Windows, easy recovery, and moderate confidence.

[LivXue]

Windows compatibility re-review: the reported native test-helper failure is fixed. No additional Windows compatibility issue was found in the inspected scope.

Scope: the complete nine-file PR at 95fbf84fe325d1fac9bb881bb2d24dcc54d419aa, base 182dacac001e6697b3260da4400d2f9e36ba536c. All 4,458 isolated snapshot blobs matched the head tree. Rechecked the complete base-to-head diff, cleanup/capture/settings callers, and three-file revision delta. Production files are byte-identical to the preceding reviewed head; this revision strengthens the tests.

All six committed Windows cleanup cases now pass on native Windows. The helper uses real taskkill where appropriate; the Python fixtures use the base interpreter and Windows-aware liveness checks, preserving launcher/worker death assertions. The two simulated taskkill timeouts exercise the launcher-only fallback. A separate native probe also passed all five real process-tree scenarios: CLI timeout, cancellation and output-sink failure, plus Kimi-helper timeout and cancellation. Each confirmed both owned launcher and worker exited, using Windows process handles, real taskkill, and a path containing spaces and non-ASCII characters.

Native Windows / Python 3.13.12. Fresh run-owned environment from uv sync --frozen --extra dev --dev, UV_NO_SYNC=1, PYTHONUTF8=1, isolated user/Raven/temp directories; imported Raven source was verified against this snapshot. Exact commands from its root:

$R = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\18f225937a1b4f799a5082d83d56d06c'
uv run pytest tests/test_subagent_third_party.py tests/test_subagent_kimi_code.py -k without_killpg -n 0 -x --tb=short --basetemp "$R\checks\927-cleanup\pytest" --junitxml "$R\checks\927-cleanup\junit.xml"
# 6 passed, 342 deselected.
uv run pytest tests/test_subagent_third_party.py -k 'login_shell_env or refresh_that_read or readers_that_find_no_capture or readers_that_wait_on_a_capture' -n 0 -x --tb=short --basetemp "$R\checks\927-capture\pytest" --junitxml "$R\checks\927-capture\junit.xml"
# 16 passed, 273 deselected.
uv run pytest tests/test_rpc_console.py tests/test_rpc_settings.py::test_secret_key_writes_string -k 'mirror or mirrored or capture or rotated_key or secret_key_writes_string' -n 0 -x --tb=short --basetemp "$R\checks\927-settings\pytest" --junitxml "$R\checks\927-settings\junit.xml"
# 26 passed, 1 skipped (real POSIX shell-mirror case), 136 deselected.
uv run pytest "$R\test_native_cleanup.py" -s -n 0 -x --tb=short --basetemp "$R\checks\927-native\pytest" --junitxml "$R\checks\927-native\junit.xml"
# 5 passed.

The three repository-test selections each emitted one idle-ceiling warning during these concurrent runs; strict idle enforcement was not enabled. No full suite, actual vendor authentication/CLI, or real POSIX login-shell integration ran. The documented limitations for an already-exited launcher, or unavailable/failed taskkill leaving workers behind, remain unchanged and are not claimed fixed by these tests.

[0xKT]

Not a blocker. Two observations from the full acceptance; neither blocks A (production is sound and your CI is green on 22 checks). I verified both on the merged tree.

Severity of this one finding, not a verdict on the pull request. Any blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text.

1. Flaky new test (worth fixing). test_an_ask_that_runs_out_takes_what_its_kimi_started (tests/test_subagent_kimi_code.py:448) sets _ASK_TIMEOUT_S = 0.3 (line 452), but 0.3s is below the time to spawn the freshly-written /bin/sh kimi stub on this host, so the stub never reaches printf $! > worker.pid before _end_tree kills the group, and line 456's int((tmp_path/"worker.pid").read_text()) raises FileNotFoundError. I reproduced it deterministically on macOS (1 failed, 58 passed); a control that changes only the timeout to 3.0 gives 59 passed. Production is unaffected (_ASK_TIMEOUT_S default is 20.0 at kimi_code.py:48, and worker.pid is test-only), and Linux CI passes because shell startup there is a few ms -- but fork+exec over 0.3s is reachable on a loaded runner, so this can flake intermittently on CI too. The sibling test at line 295 uses 1.0s; suggest matching it or higher.

2. Nit: duplicated Windows-stop rule. The same logic (if hasattr(os, "killpg"): killpg(...) else taskkill /T /F /PID) now lives in both kimi_code._end_tree (kimi_code.py:380) and cli_agent._kill_process_group (cli_agent.py:284), with no shared owner -- a later change to the Windows kill (e.g. a job-object approach) has to be made in two places. Consider one helper both call. Not blocking.

[gloryfromca]

No blockers; suggestions only, and they are marked inline.

The flaky-test observation holds and changes my stance from clean to nonblocking. The focused case passes on this Linux host (1 passed in 4.04s), but it still assumes the shell stub creates worker.pid within 0.3 seconds; the macOS reproduction shows that ordinary startup variance can violate that assumption. Production retains its 20-second timeout, so this is test reliability rather than a merge blocker. The duplicated cleanup rule is likewise a nonblocking maintainability note.

There are no review threads from my account to resolve. These two items remain named follow-ups, with the timing-dependent test the concrete one worth fixing.

[LivXue]

Confirmed, and thanks for the reproduction. #927 merged before the fix was pushed, so it is in #943.

It is a race, so the fix removes it rather than widening the window: the cap runs from the spawn and nothing tells the ask when the stand-in is ready, so a larger number only moves the threshold. Forced, it is deterministic: with the stand-in made to wait 0.5s before starting its worker, main's test fails with the same FileNotFoundError on worker.pid, and the new test passes. Here on Linux the natural start crossed 0.3s only at low priority: timing the stand-in's worker line in /bin/sh on a core shared with four busy loops, nothing crossed it at normal priority (max 194ms over 15 samples), but at nice -n 19 3 of 15 did (max 1189ms, and 4102ms with twelve busy loops).

The new test waits for ask.pid (written after the worker is noted), asserts the worker is alive, then cancels; the timeout path keeps its coverage in the tests at lines 352 and 434. The leader-only mutant (proc.kill() in place of os.killpg) still turns it red (1 failed, 347 passed over the two files), so the group-kill assertion holds. Also, "line 295" reads as line 362 or 439 here: the test at 295 sets no timeout.

[LivXue]

Agreed on the direction. With #927 and #908 both merged, main now has the rule five times: synchronous in raven/ops/transport.py and raven/utils/office.py, and async in CliAgentBackend._kill_process_group, kimi_code._end_tree and AcpClient._kill_process_tree; #930 adds a sixth, a synchronous _kill_group beside the ACP one. Merging only #927's two would not have left one owner, so I would rather do it once in a follow-up: one async helper in raven/utils/ that the three async sites call, with the synchronous pair settled at the same time and the job-object approach weighed there.