Skip to content

fix(*): end leftover ACP agents, trim the heap, and bound the session cache - #942

Merged
0xKT merged 4 commits into
mainfrom
fix/runtime_auto_cleanup
Oct 14, 2026
Merged

0xKT merged 4 commits into
mainfrom
fix/runtime_auto_cleanup

Conversation

@LivXue

@LivXue LivXue commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Summary

A long-running Raven host held on to three kinds of memory it no longer used. Observed on one dev box on 2026-10-12:

  • Two ACP agents, started on 2026-09-20 by a host that exited without closing them, were still running 22 days later. They had been reparented to init and held 523 MiB PSS between them.
  • A gateway's glibc thread arenas had grown from 85 to 214 MiB over 18 days of light use.
  • SessionManager kept every session it had ever loaded.

This PR cleans up all three automatically.

1. ACP agents that outlive their host (Linux). An ACP agent starts in a session of its own, so it gets none of the signals sent to its host's process group or terminal, and while the host lives only AcpClient.close ends it. A host that exited without closing its agents, or was killed, crashed or OOM-killed before its teardown, left them running.

  • The agent now starts through a one-line exec wrapper (raven/acp_client/leftovers.py). It runs the host's own interpreter with -I -S, arms PR_SET_PDEATHSIG(SIGKILL), checks that its parent is still the host, restores SIGPIPE and SIGXFSZ to their defaults and execs the agent in place, with the same pid and group and the agent's own argv. The agent's process then ends with the host.

  • The signal reaches only the direct child, and an npx launcher's real server is a grandchild. So every host also records the process groups it launched, one file per host under acp-launches/ in the config file's directory. The first ACP launch in any process ends the groups recorded by hosts that have since died. That sweep runs once per process on a daemon thread. Every launch waits for it for at most 5 s, and once one launch has waited that long the rest stop waiting for it.

  • A group is ended only when all of the following hold:

    • the record is of this version and comes from this boot and PID namespace;
    • its host is gone, judged by pid and start time;
    • the group is not group 0, 1 or the sweeper's own;
    • its leader, if still running, has the start time on record;
    • a current member still carries the dead host's mark, RAVEN_ACP_HOST=<host pid>-<host start ticks>, which every launched agent gets last in its environment.

    A read that fails, of a record or of /proc, keeps the record and ends nothing; a record that does not parse is deleted.

2. Native heap trimming (Linux with glibc). glibc keeps the memory native code frees, such as PyMuPDF rasters and tokenizer tables, in its arenas. raven/utils/heap.py checks every ten minutes and, once anonymous memory stands 32 MiB above its lowest reading since the last trim, runs malloc_trim(0) in a worker thread. The five long-running hosts start it: raven gateway, raven serve, the TUI's RPC host, raven acp and raven a2a serve.

3. A bounded session cache. SessionManager._cache becomes an LRU with a soft limit of 64 sessions.

  • A session leaves the cache only when nothing about it is unsaved. Its message count must match what is on disk, and a fingerprint of its metadata, consolidation mark and pending clarification must match the one taken when it was last read or written. A metadata patch counts its own keys as saved even when the caller set them on its copy first.
  • A session that a caller still holds comes back as the same object.
  • A reloaded session keeps the updated_at it was saved with.

CONTEXT.md gains two terms, Leftover agent and Launch mark.

Measured on this box on 2026-10-13:

  • PyMuPDF. 112 pages rasterised three times in a 3-thread pool. In two runs, anonymous memory was 279.8 and 315.3 MiB after store_shrink(100), and 93.5 and 84.6 MiB after heap.trim().
  • Gateway. An isolated gateway with a key-less config, run for 100 s. With the interval shortened to 45 s, one trim fired at t+47.7 s and took anonymous memory from 293 to 273 MiB; with the default interval, none fired. Final PSS was 321.6 against 328.6 MiB. Its boot check launched 16 ACP agents through the wrapper: the five discovered agents under agents/, which that config cannot start, and eleven presets, eight of which completed the handshake (Claude Code, Codex, OpenCode, Hermes Agent, OpenClaw, CodeBuddy, Qoder and Pi). Afterwards no marked process and no launch record remained.
  • Killed host. A stand-in host was SIGKILLed after launching through AcpClient. On main, a direct agent, an npx-shaped launcher and its grandchild all survive with ppid 1. On this branch, the direct agent and the launcher end with the host, and the next host's first launch ends the grandchild.
  • Discovered agents, with and without the wrapper. Each of the five under agents/ was verified twice in a home holding only dummy keys (a handshake calls no model): Raven-Design completed the handshake both ways, and the other four stopped on the same missing key both ways.

The Windows regression fixtures use explicit executable filenames and native project-path strings. The unreadable-transcript symlink fixture skips only when Windows reports missing symlink privilege (WinError 1314); other creation errors still fail. These test corrections do not extend the Linux-only cleanup mechanisms.

Type

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

Verification

The original author's Linux measurements below precede the Windows test-fixture follow-up. The native Windows results that follow were run against the final follow-up tree.

  • Author's Linux full suite before the Windows test-fixture follow-up: uv run --frozen --python 3.12 --all-extras pytest -q -p no:cacheprovider (repo addopts: -n 4, integration and e2e deselected). Result: 3 failed, 30261 passed, 158 skipped, 2 xfailed. The failures are the ones this box gives on main too: test_install_script.py::test_resolve_node_dir_answers_each_case_it_exists_for, test_knowledge_parser_excel.py::test_a_legacy_workbook_is_converted_and_read (no LibreOffice) and test_subagent_node_runtime.py::test_what_cannot_be_read_names_nothing (run as root).
  • Directly affected files: tests/test_acp_leftovers.py, tests/test_utils_heap.py, tests/test_session_manager.py, tests/test_cli_{a2a,acp,gateway,serve,tui}_commands.py and tests/test_subagent_host_env.py, under --idle-ceiling-strict as CI runs them. Result: 620 passed.
  • Neighbours: tests/test_rpc_session.py, tests/test_session_{export,title,project_grouping}.py, tests/test_defer_manager.py, tests/test_subagent_acp.py and tests/test_acp_{ask_user,elicitation,remote}.py. Result: 683 passed.
  • Lint and gates: make lint-python lint-imports lint-deps lint-types check-large-files check-source-language check-commits. Result: all seven exit 0; import contracts 10 kept, 0 broken.
  • Mutation: guards were deleted or inverted one at a time, out of tree, and each such mutant failed the test written for it. They covered the sweep's wait, bound, daemon flag and start failure; the version, boot-time, PID namespace, liveness, leader and mark checks; the /proc/<pid>/stat parse; the trimmer's reader and threshold; and the cache's eviction order, fingerprint, updated_at restore and patch rule.
  • Diff coverage: 96.27% (387 of 402 executable changed lines) from the directly affected and neighbouring files alone, against CI's 90% gate. What stays uncovered is the arms for a /proc read or a killpg that fails, and the import inside raven a2a serve, which only a running server reaches.

Native Windows test-fixture follow-up (2026-10-14): Windows 10.0.26300, Python 3.13.12, PYTHONUTF8=1, a dedicated worktree and uv sync --frozen --python 3.13 --all-packages --extra dev. The imported Raven package was verified to come from that worktree. Pytest's isolated homes and a separate caller RAVEN_HOME were used. JUnit output paths are omitted from the command below.

uv run --frozen --no-sync pytest tests/test_acp_leftovers.py tests/test_utils_heap.py tests/test_session_manager.py tests/test_session_project_grouping.py tests/test_subagent_host_env.py tests/test_cli_a2a_commands.py::test_serve_starts_the_heap_trimmer tests/test_cli_acp_commands.py::TestServe::test_the_heap_trimmer_starts_before_the_channel_is_served tests/test_cli_gateway_commands.py::test_run_starts_the_heap_trimmer_at_boot tests/test_cli_serve_commands.py::test_serve_starts_the_heap_trimmer_at_boot tests/test_cli_tui_commands.py::test_the_terminal_host_starts_the_heap_trimmer -k 'not test_acp_child_inherits_custom_raven_home and not test_acp_child_is_told_it_serves_as_a_subagent' -n 0 --idle-ceiling-strict --tb=short

Result: 229 passed, 73 skipped, 2 deselected, exit 0 with the idle ceiling enforced. Of the skips, 72 are existing Linux/glibc cases and one is the measured lack of Windows symlink privilege. The three corrected discovery/path cases failed before the corrections and pass in this selection. Before the symlink guard, the same broader selection had 229 passes, 72 skips, two deselections and one WinError 1314 failure.

The two deselected real ACP child tests still depend on the native close fix in #908. The broader CLI permission/signal limitations reported in the Windows review are covered separately by #939. This follow-up claims no full Windows suite pass, real ACP lifecycle validation, privileged symlink run, Windows ACL audit, or new Linux run.

  • uv run --frozen --no-sync ruff check raven evolver agents plugins-dist tests scripts docs-site and the matching ruff format --check: passed (2,272 files formatted); the final symlink edit also passed both file-scoped checks.
  • uv run --frozen --no-sync lint-imports: 10 contracts kept. uv run --frozen --no-sync ty check raven/acp_client/client.py raven/acp_client/leftovers.py raven/cli/a2a_commands.py raven/cli/acp_commands.py raven/cli/gateway_commands.py raven/cli/serve_commands.py raven/cli/tui_commands.py raven/session/manager.py raven/utils/heap.py: passed.
  • uv run --frozen --no-sync pre-commit run --from-ref origin/main --to-ref HEAD: all applicable hooks passed.
  • node node_modules/@commitlint/cli/cli.js --from origin/main --to HEAD --config commitlint.config.cjs, uv run --frozen --no-sync python scripts/check_commit_messages.py origin/main..HEAD, uv run --frozen --no-sync python scripts/check_large_files.py origin/main...HEAD, uv run --frozen --no-sync python scripts/check_source_language.py origin/main...HEAD, and git diff --check origin/main...HEAD: passed.

The relevant-tests checkbox remains unchecked because the broader Windows files still contain the disclosed limitations above.

  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

No user-facing docs change, because no command, flag or setting does. The two new terms are glossary entries in CONTEXT.md.

Risk

User-visible behaviour changes:

  • An ACP agent that exists but cannot be executed now fails after the spawn. Examples are a bad interpreter line or a binary for the wrong architecture. The wrapper writes raven: cannot start <path>: <error> to the agent's stderr and exits 127, so the failure surfaces as a connection that ends with that line in its stderr, where AcpClient.launch used to raise cannot start at once. A missing program still fails at launch, as before.
  • Agent environment. Every ACP agent's environment carries RAVEN_ACP_HOST. It is an identifier, not a secret; its parts are world-readable in /proc/<pid>/stat.
  • Locale. An agent started under a C or POSIX locale gets LC_CTYPE=C.UTF-8, because the wrapper interpreter's PEP 538 coercion carries through the exec.
  • Launch records. Every ACP launch and close writes a small JSON file under acp-launches/ beside the config file, synchronously and with an fsync.
  • First launches. While dead hosts' records are swept, a process's ACP launches wait for the sweep, 5 s at most in all: once one launch has waited that long, the rest start without it. Normally the sweep takes milliseconds.
  • Session timestamps. A reloaded session's updated_at is the one it was saved with, where it used to be the time of the load, so a save that adds no message no longer moves it. Sentinel's defer timing and the export header read it in memory, and the session list orders by the saved value.

Left as today:

  • Anything but Linux. fix(acp_client): on macOS and Windows an ACP agent outlives a host that dies without closing it #928 tracks macOS and Windows.
  • A group whose surviving processes carry no readable mark: one started with a fresh environment, another user's, or an undumpable one.
  • An agent binary that is set-id or carries file capabilities: its exec clears the parent-death signal, so only the sweep can end it, and only where its environment is readable.
  • Spawns other than ACP agents: the CLI sub-agent backend, background exec and the sandbox executor.
  • Leftovers created during one long-lived host's life. They wait for another process's first ACP launch.
  • A host started with another --config sweeps that directory's records only.

Edge cases of the records directory:

  • Another PID namespace. Its records are kept until a reboot, about 300 bytes per container start.
  • A directory shared between machines. Another machine's record older than this machine's boot is removed by this machine's sweep. That host's agents would then not be swept if it died before its next launch or close rewrote the file. Nothing is ever signalled across machines.

Edge cases of the session cache:

  • A change made through a reference to a session that had already left the cache, and then released unsaved, is lost, as it would be in a fresh process.
  • An in-memory undo whose save failed, followed by new messages back to the same count, reads as clean. save() already mishandles that state, so the residue predates this PR.

A group is signalled only when it passes all five checks above. A crafted record cannot point a sweep at a live host's agents, because the mark names the identity the liveness check reads.

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

Rollback is a revert. Nothing persisted changes shape: session files are written exactly as before, and acp-launches/ only holds what the next sweep reads, so it can be deleted at any time.

Related Issues

#928 (macOS and Windows; stays open)

… cache

A long-running host kept memory it no longer used: ACP agents of a host
that exited without closing them, native memory glibc keeps in its
arenas, and every session SessionManager had ever loaded.

On Linux an ACP agent now starts through an exec wrapper that arms
PR_SET_PDEATHSIG, and every host records the process groups it launched
under acp-launches/ beside the config file. The first ACP launch in any
process ends the groups of hosts that have since died: only in the same
boot and PID namespace, only for a record of this version, and only
where a member still carries the dead host's RAVEN_ACP_HOST mark.

The gateway, raven serve, the TUI's RPC host, raven acp and raven a2a
serve run malloc_trim(0) in a worker thread, at most every ten minutes,
once anonymous memory stands 32 MiB above its lowest since the last
trim.

SessionManager's cache becomes an LRU with a soft limit of 64. Only a
session with nothing unsaved leaves it, one a caller still holds comes
back as the same object, and a reload keeps the saved updated_at.
@LivXue
LivXue requested review from 0xKT and gloryfromca October 13, 2026 16:38

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

I reviewed the full github/main...HEAD diff and the relevant surrounding code and history. I covered the repository rules in AGENTS.md/CLAUDE.md and CONTEXT-MAP.md, the ACP launch/close callers and Linux process-identity safeguards, the heap-trimmer host integrations, session cache/dirty-state behavior and metadata-patch interactions, backward compatibility, architecture import boundaries, and the tests for signs of weakened coverage. I found no concrete failure worth raising.

Verification:

  • uv sync --all-packages (the first test attempt could not enter tests because the workspace plugin raven_everos was not installed)
  • uv run pytest tests/test_acp_leftovers.py tests/test_utils_heap.py tests/test_session_manager.py tests/test_cli_a2a_commands.py tests/test_cli_acp_commands.py tests/test_cli_gateway_commands.py tests/test_cli_serve_commands.py tests/test_cli_tui_commands.py tests/test_subagent_host_env.py -q - 620 passed
  • uv run ruff check ... over the changed Python files - passed
  • uv run lint-imports - 10 contracts kept
  • source-language and large-file checks over github/main...HEAD - passed

@LivXue

LivXue commented Oct 13, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility review: head 43f79c1df42bd881d857a68e486f6d1a1d3b78d6, base efe0f46aed75f0f25666f8eba1f1f1b1ed231011.

No Windows compatibility regression found in the inspected changes. Reviewed all 19 changed files, ACP launch/close callers, all five host integrations, session persistence/locking and timestamp consumers, tests, and the existing review.

Native Windows / Python 3.13.12 verification:

  • Focused changed-behavior selection: 170 passed, 72 skipped (Linux/glibc mechanisms).
  • Two external scratch probes: 2 passed. They checked the real Windows platform guards without overriding the platform, and 70-session eviction/reload, UTF-8/CRLF, metadata paths, atomic rewrite and deletion beneath a directory containing spaces and Chinese characters. The Linux wrapper/sweep and heap trimmer remained inactive.
  • The argv tests use a recording subprocess substitute; they do not establish a real ACP agent lifecycle.

Broader file-level runs were not green. Each failure below was reproduced on the exact base as well as this head:

Existing test limitation Observed result on both revisions
test_subagent_host_env.py::test_acp_child_inherits_custom_raven_home AcpClient.close() calls unavailable os.killpg (head line 329, base line 315).
test_cli_serve_commands.py::TestTheSessionSurvivesARestart::test_the_state_file_stays_owner_only Windows reports mode 0666 rather than the asserted POSIX 0600; this does not establish an ACL exposure.
test_cli_serve_commands.py::TestTheSupervisorCleansUpOnTheSignalThatStopsIt::test_sigterm_removes_the_state_file Fixture launches sleep 600; native spawn returns WinError 2 before the expected state file exists.
test_session_project_grouping.py::test_slug_collides_for_a_separator_and_an_underscore Windows backslashes differ from the literal POSIX path expectation.
test_subagent_host_env.py::test_windows_probe_finds_agents_on_the_captured_path Extensionless shell-script fixtures are reported missing.

The initial broader run also stalled in TestOpenStdin::test_it_reads_the_process_stdin and was terminated. Code comparison confirms that class, _open_stdin and its feeder are unchanged from base; this is code evidence, not a second baseline hang reproduction.

Verification commands

Run from isolated detached head/base checkouts. Setup succeeded with uv sync --frozen --python 3.13 --all-packages --extra dev. Tests used PYTHONUTF8=1, UV_NO_SYNC=1, the isolated environment, and separate home/temp/token directories. Below, $run is the review's temporary directory and $suite selects a separate evidence directory.

$run = 'C:/Users/livxu/AppData/Local/Temp/raven-windows-review/f3a5f15497914585b2ca031de778aff2'
$cases = @(
  'tests/test_acp_leftovers.py',
  'tests/test_utils_heap.py',
  'tests/test_session_manager.py',
  'tests/test_cli_a2a_commands.py::test_serve_starts_the_heap_trimmer',
  'tests/test_cli_acp_commands.py::TestServe::test_the_heap_trimmer_starts_before_the_channel_is_served',
  'tests/test_cli_gateway_commands.py::test_run_starts_the_heap_trimmer_at_boot',
  'tests/test_cli_serve_commands.py::test_serve_starts_the_heap_trimmer_at_boot',
  'tests/test_cli_tui_commands.py::test_the_terminal_host_starts_the_heap_trimmer',
  'tests/test_subagent_host_env.py::test_acp_launch_preserves_stored_windows_path_quotes',
  'tests/test_subagent_host_env.py::test_on_windows_a_bare_program_starts_from_the_childs_own_path',
  'tests/test_subagent_host_env.py::test_on_windows_an_acp_server_that_is_a_batch_file_starts_from_the_childs_path'
)
$suite = '942-changed'
uv run pytest @cases -q -n 0 -x --tb=short --basetemp "$run/checks/$suite/pytest" --junitxml "$run/checks/$suite/junit.xml"
# 170 passed, 72 skipped

$suite = '942-native'
uv run pytest "$run/probes/test_windows_pr942.py" -q -n 0 -x --tb=short --basetemp "$run/checks/$suite/pytest" --junitxml "$run/checks/$suite/junit.xml"
# 2 passed

Each baseline control used uv run pytest "tests/$node" -q -n 0 -x --tb=short --basetemp "$run/checks/$suite/pytest" --junitxml "$run/checks/$suite/junit.xml", with $node equal to the corresponding table entry and suites 942-base-env, 942-base-mode, 942-base-signals, 942-base-slashes, and 942-base-probe. Each stopped at the same failure shown above.

Coverage limits: no full-suite pass, Linux parent-death/real heap-trim execution, real ACP product launch, or live UNC-share test. The Linux-only scope and remaining Windows work in #928 remain relevant.

@LivXue

LivXue commented Oct 13, 2026

Copy link
Copy Markdown
Member Author

Addressed the inherited Windows test-fixture limitations from the native review:

  • 4581fb927159: compare persisted project directories with native Path strings. This fixes the reported collision assertion and the adjacent project-origin assertion.
  • 423e2ef4320d: use discoverable .exe placeholders through the existing fixture helper. Both the boot PATH and refreshed registry PATH assertions remain; the probe does not execute these files.
  • 072257041533: skip the unreadable-transcript symlink fixture only for Windows WinError 1314; other creation errors still fail.

Native Windows 10.0.26300 / Python 3.13.12, PYTHONUTF8=1, isolated Raven/pytest homes, and imports verified against the dedicated worktree. Setup: uv sync --frozen --python 3.13 --all-packages --extra dev.

The original discovery test and both path assertions failed before the corrections. The final regression command (JUnit output path omitted) was:

uv run --frozen --no-sync pytest tests/test_acp_leftovers.py tests/test_utils_heap.py tests/test_session_manager.py tests/test_session_project_grouping.py tests/test_subagent_host_env.py tests/test_cli_a2a_commands.py::test_serve_starts_the_heap_trimmer tests/test_cli_acp_commands.py::TestServe::test_the_heap_trimmer_starts_before_the_channel_is_served tests/test_cli_gateway_commands.py::test_run_starts_the_heap_trimmer_at_boot tests/test_cli_serve_commands.py::test_serve_starts_the_heap_trimmer_at_boot tests/test_cli_tui_commands.py::test_the_terminal_host_starts_the_heap_trimmer -k 'not test_acp_child_inherits_custom_raven_home and not test_acp_child_is_told_it_serves_as_a_subagent' -n 0 --idle-ceiling-strict --tb=short

229 passed, 73 skipped, 2 deselected; exit 0. Skips: 72 existing Linux/glibc cases and one measured Windows symlink-privilege limitation. The two real ACP child cases remain dependent on #908; the broader CLI mode/signal cases are covered separately by #939. No full Windows suite, privileged symlink run, ACL audit, real ACP lifecycle, or new Linux execution is claimed. The Linux-only cleanup scope remains unchanged.

Ruff, 10 import contracts, changed-production type checks, full-PR pre-commit, commitlint, commit/source-language/asset gates and the complete-diff review passed. The description now separates the original Linux measurements from this Windows follow-up. The exact tested commits were published through a guarded fast-forward ref update and read back from GitHub; the new CI run is pending.

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

I reviewed the three test-only commits since the previously reviewed head. The native-path expectations match the production str(Path(...)) behavior, the .exe fixtures preserve the boot-PATH and refreshed-registry-PATH assertions on both simulated and native Windows, and the symlink test skips only for Windows privilege error 1314 while re-raising unrelated failures. I also checked that the changed tests still exercise their original behavior rather than weakening it.

Verification:

  • uv run pytest tests/test_session_project_grouping.py tests/test_subagent_host_env.py -q - 66 passed
  • uv run ruff check tests/test_session_project_grouping.py tests/test_subagent_host_env.py - passed
  • source-language check over github/main...HEAD - passed

@LivXue

LivXue commented Oct 13, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review at 07225704153325c48464bba06e8f4283779c7212 (base efe0f46aed75f0f25666f8eba1f1f1b1ed231011): no additional Windows issue found in the inspected changes.

Rechecked the complete 20-file PR scope and its callers against the previously reviewed tree. Only the two test files changed since 43f79c1df42bd881d857a68e486f6d1a1d3b78d6; runtime code is identical. The native path expectations and explicit .exe discovery fixtures now pass. The unreadable-transcript test skips specifically for missing Windows symlink privilege (WinError 1314), while other setup errors still propagate.

Native Windows / Python 3.13.12, an exact detached checkout, fresh isolated uv environment and separate home/temp/token directories; imports verified at this head. Setup uv sync --frozen --python 3.13 --all-packages --extra dev succeeded. With PYTHONUTF8=1 and UV_NO_SYNC=1:

$R = 'C:/Users/livxu/AppData/Local/Temp/raven-windows-review/c281ddf96c3446c98e4f4127722b59c6'
uv run pytest tests/test_acp_leftovers.py tests/test_utils_heap.py tests/test_session_manager.py tests/test_session_project_grouping.py tests/test_subagent_host_env.py tests/test_cli_a2a_commands.py::test_serve_starts_the_heap_trimmer tests/test_cli_acp_commands.py::TestServe::test_the_heap_trimmer_starts_before_the_channel_is_served tests/test_cli_gateway_commands.py::test_run_starts_the_heap_trimmer_at_boot tests/test_cli_serve_commands.py::test_serve_starts_the_heap_trimmer_at_boot tests/test_cli_tui_commands.py::test_the_terminal_host_starts_the_heap_trimmer -k 'not test_acp_child_inherits_custom_raven_home and not test_acp_child_is_told_it_serves_as_a_subagent' --idle-ceiling-strict -q -n 0 -x --tb=short --basetemp "$R/checks/942-regression/pytest" --junitxml "$R/checks/942-regression/junit.xml"
# 229 passed, 73 skipped, 2 deselected; exit 0

The 73 skips are 72 Linux/glibc cases and one unavailable symlink privilege. The two real ACP-child tests remain excluded because the unchanged Windows close limitation was reproduced at both head and base in the previous review. Its broader CLI pipe/mode/signal limits also remain outside this passing selection.

This verifies the fixture corrections and selected host behavior; it does not establish Windows parent-death cleanup, Linux heap-trim behavior, real ACP product lifecycle, privileged symlinks, ACL enforcement, or a full-suite pass.

@0xKT
0xKT merged commit 2cf8b8a into main Oct 14, 2026
22 checks passed
@0xKT
0xKT deleted the fix/runtime_auto_cleanup branch October 14, 2026 03:06
LivXue added a commit that referenced this pull request Oct 14, 2026
…iled (#944)

## Summary

`SessionManager.save()` lost the turns that follow an undo or a clear
whose own save failed, and brought the cut turn back.

`save()` appends `messages[_persisted_count:]`, and rewrites the file
only while the list is shorter than `_persisted_count`.
`Session.undo_last_turn()` and `Session.clear()` cut the list in memory
and leave the write to their callers. When that write fails, the file
keeps the cut messages and `_persisted_count` keeps its old value. Once
new messages take the list back to that count, the next save appends
past it: the file keeps the cut messages, and the ones that replaced
them are never written. Past the old count, the file ends up holding the
cut messages followed by the tail of the new ones.

Where such a failed save comes from:

- `session.undo` (the TUI's `/undo` and `/retry`) and `session.clear`
save right after the cut and only log a failure.
- `/new` saves after `clear()` and raises on a failure before it
invalidates the session, so the cleared copy stays cached.

So a `/retry` after an undo whose save failed loses the turn it sent
again on the next reload, and the undone turn comes back. The bounded
session cache from #942 read that state as saved too, since its clean
check compares the message count and a fingerprint of the other saved
fields, so such a session could leave the cache and be reloaded from the
stale file.

`Session.truncations` already counts every cut: both methods bump it,
and the episodic annotator reads it for the same reason, since a cut is
invisible to indices once the list has grown back.
`Session._persisted_truncations` holds that count as of the last time
this copy read the file or wrote it successfully. Three readers treat a
difference as unsaved:

- `save()` rewrites the file atomically, however far the list has grown
back.
- The cache's clean check keeps the session cached.
- `flush()`, which `session.close` calls, saves it.

`session.close`'s docstring names the new case. Nothing on the wire or
on disk changes shape.

## Type

- [x] Fix
- [ ] Feature
- [ ] Docs
- [ ] CI / tooling
- [ ] Refactor
- [ ] Other

## Verification

- **New tests, against the unfixed code:** six of the seven new cases
failed there, each reading back the file as the cut left it, for example
`['q1', 'a1', 'q2', 'a2']` where the retried answer belonged. The
seventh,
`test_once_a_cut_is_on_disk_a_save_keeps_what_another_writer_appended`,
passes there by design: it holds the append path for concurrent writers
once a cut is on disk. All seven pass with the fix.
- **Mutation:** each construct was removed or moved one at a time, with
the backup outside the tree, and each mutant failed the tests written
for it: `save()` ignoring the cut (4 failed), the counter never recorded
(1, the concurrent-writer case), the clean check ignoring the cut (1),
`flush()` ignoring it (1), and the counter recorded before the write
instead of after it (6).
- **Directly affected and neighbouring files:** `uv run --frozen
--python 3.12 --all-extras pytest -q -p no:cacheprovider
--idle-ceiling-strict` over `tests/test_session_manager.py`,
`tests/test_rpc_session.py`, `tests/test_rpc_schema_match.py`,
`tests/test_rpc_session_init_bundle.py`,
`tests/test_rpc_session_naming.py`,
`tests/test_session_{export,project_grouping,title}.py`,
`tests/test_defer_manager.py` and
`tests/test_agent_loop_{episodic_annotate,empty_recovery}.py`. Result at
this head: 1003 passed.
- **Full suite at this head:** `uv run --frozen --python 3.12
--all-extras pytest -q -p no:cacheprovider` (repo addopts: `-n 4`,
integration and e2e deselected). Result: 8 failed, 31091 passed, 164
skipped, 2 xfailed. The failures are this box's environment, none
introduced: five in `tests/test_config_update_providers.py` follow the
shell's proxy variables (that file passes 193 of 193 with them unset),
and
`test_install_script.py::test_resolve_node_dir_answers_each_case_it_exists_for`,
`test_knowledge_parser_excel.py::test_a_legacy_workbook_is_converted_and_read`
(no LibreOffice) and
`test_subagent_node_runtime.py::test_what_cannot_be_read_names_nothing`
(run as root) fail the same way on an exported copy of the base commit.
- **Diff coverage:** the seven executable changed lines in
`raven/session/manager.py` are all executed by
`tests/test_session_manager.py` and `tests/test_rpc_session.py`.
- **Lint and gates:** `make lint-python lint-imports lint-deps
lint-types check-large-files check-source-language check-commits`.
Result: all seven exit 0; import contracts 10 kept, 0 broken.

- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [ ] User-facing docs or screenshots are updated when needed

No user-facing docs change, because no command, flag, setting or file
format does.

## Risk

User-visible behaviour changes:

- **A save after a cut whose own save failed now rewrites the file.** A
rewrite writes this copy's messages only, so turns another process
appended to the same session since this copy last read it are dropped,
as they already are when a cut is saved on the first try. Before this
change that save appended: it kept those turns, and left the cut
messages in place of the new ones.
- **`session.close` writes a pending cut.** `flush()` used to skip a
session whose list was not longer than the saved count.
- **A session with a pending cut stays in the bounded cache** until a
save succeeds. The limit is soft, as it already is for any session with
unsaved changes.

Left as today:

- `session.undo` and `session.clear` still return success when their own
save fails, and only log it. The cut now reaches the disk with the next
successful save, and is lost if the process ends first.

- [x] Security impact considered
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes

Rollback is a revert. Session files are written in the same format as
before, and the new counter is never written to disk.

## Related Issues

#942 (its Risk section disclosed the cache half of this)
LivXue added a commit that referenced this pull request Oct 15, 2026
… dead host left (#953)

## Summary

On macOS an ACP agent outlives a Raven host that dies without closing it (killed, crashed, OOM-killed). The agent runs in a session of its own, so it gets none of the host's signals, and only `AcpClient.close` ends it. #942 closed this on Linux with a parent-death wrapper and a launch-record sweep. On macOS both were no-ops. This PR brings both layers to macOS.

**1. The agent ends with its host (`raven/acp_client/leftovers.py`, `_DARWIN_BINDING`).** macOS has no `PR_SET_PDEATHSIG`. Instead, the exec wrapper double-forks a small watcher before it execs the agent, then reaps the middle child.
- The watcher is adopted by launchd, so the agent has no child it did not start.
- It stays in the agent's process group, closes every descriptor it inherited (so it holds none of the agent's pipes), and changes to `/`.
- It waits on the host's exit with kqueue (`EVFILT_PROC` / `NOTE_EXIT`), then ends the whole group. An `npx` launcher's grandchild goes with it, which the Linux signal does not reach.
- If the host is already gone when the watch is set, registration fails with `ESRCH`, and that counts as an exit.
- It ends the group only while the agent leads its own group, which every launch does. Any other failure leaves the agent as it would have been.
- `close()` ends the watcher along with the rest of the group.
- Cost: about 4 MiB RSS per launched agent.

**2. The sweep at the next launch runs on macOS.** This is the backstop for when the watcher itself is gone. The decision logic is unchanged; each `/proc` read has a macOS equivalent:
- process start time: `sysctl KERN_PROC_PID` (`kinfo_proc.p_starttime`, in microseconds). It reads any process, including launchd and zombies; `proc_pidinfo` refuses launchd with EPERM.
- group members: `proc_listpids(PROC_PGRP_ONLY)`
- launch mark: `sysctl KERN_PROCARGS2`. EINVAL (gone, zombie, or another user's process) counts as no mark.
- boot identity: `kern.bootsessionuuid` and `kern.boottime`. macOS has no PID namespaces, so the namespace is a fixed value.

macOS hides the environment of its own platform binaries (`/bin/sh`, `/bin/sleep`). Only a group member that is not one can show the mark, such as the agent's node or python process, or the watcher. What cannot be read still leaves a group alone.

**Linux is unchanged.** The Linux wrapper string is byte-identical to main's `_WRAPPER`. `_start_ticks` is renamed to `_start_time`, which now dispatches by platform. `CONTEXT.md` (Leftover agent, Launch mark) is updated to match.

**Windows lifetime handling is provided by #951.** This PR adds the macOS wrapper and sweep; these helpers remain a no-op on Windows.

## Type

- [x] Fix
- [ ] Feature
- [ ] Docs
- [ ] CI / tooling
- [ ] Refactor
- [ ] Other

## Verification

Before rebasing onto the merged Windows lifetime change, on macOS 26.6.2, arm64, Python 3.12 (project venv):

- `uv run pytest tests/test_acp_leftovers.py tests/test_subagent_host_env.py tests/test_acp_client_teardown.py tests/test_acp_client_launch.py -q -p no:cacheprovider -m "slow or not slow"` -> 117 passed, 9 skipped (Linux-only mechanisms).
- Every other `tests/test_*acp*.py` and `tests/test_*subagent*.py` file, run one file at a time -> all pass except `tests/test_subagent_dag_runner.py`. On this Mac that file hangs at the same two points with this branch and with main's `leftovers.py`:
  - after `TestBackgroundRun::test_a_run_that_ignores_cancellation_fails_named_instead_of_hanging_loop_close` fails;
  - with that test deselected, after `TestBackgroundRun::test_the_failure_path_reaps_a_subagent_child_left_running` (21 passed in both runs).
  It is unrelated to this change; Linux CI covers that file.
- Red/green with a stand-in host (`AcpClient.launch`, then `kill -9` of the host). The stand-in agent ignores SIGTERM, never reads stdin, and starts a child of its own.
  - main: 3 s later the agent is alive with ppid 1, and its child is alive.
  - this branch: 3 s later both are gone.
- Sweep backstop: kill the watcher, then `kill -9` the host. The agent stays (ppid 1). The next host's launch ends the agent and its child.
- `uv run ruff format --check` and `uv run ruff check` on the changed files -> clean. `make check-source-language` and `make check-large-files` -> clean.

New macOS tests cover:
- a killed host ending the agent and what it started;
- the watcher holding none of the agent's pipes and being the only member left after the agent exits;
- the watcher ending nothing when the agent does not lead its group;
- the exec keeping the agent's pid and argv;
- SIGPIPE and SIGXFSZ reaching the agent at their defaults;
- the start-time, member, environment and boot readers (including an empty argv entry and a platform binary).

Most existing Linux tests now run on macOS too.

After rebasing onto the merged Windows lifetime change, on Linux / Python 3.12:

- The only manual conflict resolution is in `CONTEXT.md`: retain both platform descriptions, scope the record sweep to Linux and macOS, and remove the stale sentence that macOS only has explicit teardown.
- The macOS implementation and its tests are byte-identical to the approved revision. The Windows implementation and its tests are byte-identical to main; the shared client and environment-test logic also match main, with only the macOS comments/docstring added.
- `uv run --no-sync pytest tests/test_acp_leftovers.py tests/test_acp_windows.py tests/test_acp_client_launch.py tests/test_acp_client_teardown.py tests/test_subagent_host_env.py tests/test_subagent_acp.py tests/test_living_docs.py -q -x` -> 468 passed, 10 macOS-only tests skipped.
- `uv run --no-sync ty check raven/acp_client` and `uv run --no-sync lint-imports` -> passed; all 10 import contracts kept.
- `uv run --no-sync pre-commit run --from-ref origin/main --to-ref HEAD` -> passed for every applicable hook.
- `make check-large-files check-source-language check-commits` with the existing environment and `UV_NO_SYNC=1` -> passed.

Native macOS and Windows tests were not repeated after this documentation-only conflict resolution. The current-head Windows re-review before the rebase reported 65 passed and 79 platform-specific skips.

- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [x] User-facing docs or screenshots are updated when needed

## Risk

- User-visible: on macOS an ACP agent is now SIGKILLed with its whole process group when its host exits without closing it. On Linux the parent-death signal ends the direct child; the launch-record sweep handles surviving marked descendants. An agent that relied on outliving its host no longer does.
- Each launched agent on macOS carries one extra idle process (about 4 MiB) in its group until the group is closed or the host exits.
- The sweep only acts on groups recorded by a dead host in the same boot, whose leader start time matches and which still hold a member carrying that host's mark. A recycled pid or group number is left alone.
- Rollback: revert this commit. Record files a macOS host left under `acp-launches/` are then inert, because the sweep is a no-op on macOS again.

- [x] Security impact considered
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes

## Related Issues

#928 (macOS); #951 provides the Windows lifetime handling.
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.

3 participants