Skip to content

perf(*): let raven agents share memory with their host and with each other - #1127

Open
LivXue wants to merge 19 commits into
mainfrom
perf/share_subagent_child_memory
Open

LivXue wants to merge 19 commits into
mainfrom
perf/share_subagent_child_memory

Conversation

@LivXue

@LivXue LivXue commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Summary

Each product agent under agents/ (Raven-Code, -Design, -Oncall, -PPT, -Research) ran as a whole Raven process of its own: about 240 MiB PSS, nearly all of it private. A host running the five products paid about 1.2 GiB for them. This lets those agents share memory with their host and with each other through three mechanisms, each switchable live under subagents.runtime in the host's config.

  • idleCloseS (default 600): the ACP pool closes an agent's process once nothing is in flight either way and the agent has been silent for the budget. Only an agent that advertises loadSession, or one that never opened a session, is closed, so a stateful instance's next turn reopens its session in a fresh process. A held wake is a prompt still waiting for its answer, so it never reads as idle. Before closing a Raven agent the pool asks it (_raven/residentWork, declared in agentCapabilities._meta) whether it holds resident work, an armed cron job of its own partition or a running plugin service, and keeps it while it does; an agent that never declared it, or does not answer in time, is closed as before. Each event loop closes only the connections it made.
  • llmRelay (default on): the host makes a Raven agent's LiteLLM calls, so the agent never imports LiteLLM (about 120 MiB of it). LiteLLMProvider's LiteLLM-free construction and answers move to a new base, LiteLLMShape, which the agent's RelayProvider shares, so both answer every question from one copy of each rule. Only the single chat / chat_stream attempt crosses to the host, sized under the agent's own turn window and run in a context of its own there, plus the catalogue lookups that need LiteLLM's tables (marked with the relayed decorator in rates, capabilities, common_models and the Anthropic adapter). Transport: a Unix socket in a private directory (loopback TCP where the system has no Unix sockets, as on Windows) and a per-host token, presented first and under a deadline before any frame is read. Retry, classification, usage records and traces stay in the agent; the host's calls run under recorded_by_caller, so nothing is billed twice. An agent that cannot reach its host, or is offered a Unix socket on a system without them, loads LiteLLM as before. A relayed call is made with the host's environment, so an agent whose own configured env sets an LLM endpoint, a cloud credential, a proxy or a CA bundle is launched without the relay; and a refusal of prompt-cache marks is learned on the host as well, since the host builds the agent's resend. A section's tls_verify opt-out crosses with the construction, so a relayed call to a self-signed gateway is made on the host without certificate checks, as the agent's own call would be.
  • forkTemplate (default on, Linux only): a template process that has already imported what agents import forks each Raven agent the host launches, so the loaded pages are shared. It preloads the modules at least two products import, learned per product from the agents themselves (one product's own set while only one has been seen), plus the token estimator's table. A product launcher's os.execv of python -m raven runs Raven in the same process. The template ends with the host and its agents end with it. Anything else, and any failure, launches the ordinary way. A template that has not reported ready within a minute, as on a busy machine, is ended and a new one is tried ten minutes later; one that cannot serve is not used again. The template's own process is a launcher beside the CLI, raven/cli/_fork_template.py, because it loads the surfaces by name and runs python -m raven. The ACP client keeps only the host's side in raven/acp_client/fork_template.py and starts the template through Raven's entry point (python -m raven --fork-template), which settles the certificate store before the template imports anything, so a forked agent's TLS contexts trust what a launched agent's do.

Measured with a stub LLM endpoint, one turn per agent, PSS in MiB, on the branch's first version:

today relay only relay + template
five agents, total 1414 959 724
five agents, the agents alone 1259 783 547
one agent, total 402 328 371

At this head, five agents with both on measured 735 MiB in total, with module lists learned on an earlier version. The first run after a code change measures about 794: each agent compiles the bytecode of the modules that changed. With a single agent the template costs about 40 MiB more than the relay alone: the pages an agent writes to are copied, and the originals stay in the template. Two or more agents at a time come out ahead. The defaults stay on; a device that only ever runs one agent can set forkTemplate to false.

Other changes that come with it:

  • The relay and the template belong to the event loop they started under. A CLI turn is one asyncio.run, and a relay left by an ended loop accepts connections nobody answers. The template is closed and reaped by its own loop as that run ends, and its pipes are closed there even when the close is cut short. A loop running beside them in another thread (the RPC server runs a CLI command such as raven agent -m on a worker thread) leaves them alone: it launches its agents the ordinary way and is offered the same relay socket.
  • raven acp takes the relay offer out of its environment at startup, so an MCP server or shell command it starts does not inherit the token.
  • The token estimator drops the duplicate copy of its ranks itself. LiteLLM's import used to do that, and a relayed agent no longer imports LiteLLM.
  • New terms in CONTEXT.md: Idle close, LLM relay, Fork template. The settings are documented on the agent integrations page, in English and Chinese.

Not covered: agents on another machine, one-shot CLI hosting (--task), and third-party ACP agents launch as before. Not measured on Windows or macOS.

Pre-existing on main and not touched here: under xdist, tests/test_agents_oncall_flow_submit_tool.py can fail when a worker has already run tests/test_agent_plugins_ledger.py::test_load_ignores_a_file_that_is_not_an_object. The oncall duplicate check scans two levels above its ledger, which is the worker's temp root, and reads that test's list-shaped ledger.json. Reproduced on main by running the two files in that order.

Type

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

Performance (commit type perf).

Verification

  • pytest -q -n 4 under Python 3.12 at this head, rebased onto b05267b2e: 37349 passed, 255 skipped, 8 failed. The 8 fail here only because of this machine (5 proxy tests in test_config_update_providers.py that read the shell's proxy variables, 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, which fails on main here too, and test_subagent_node_runtime.py::test_what_cannot_be_read_names_nothing, which needs a non-root user). The oncall ordering failure described above depends on how xdist distributes the files and did not occur. On 9d9bec90b, before the template fix in this head, one of six full runs failed a ninth test, test_acp_fork_template.py::test_an_acp_launch_of_a_raven_agent_is_forked_and_offered_the_relay: the template wrote nothing within its 60 s ready deadline, so the agent was launched the ordinary way. The other five passed, as did the test alone (3 of 3) and with its file (28 of 28). The template was slow rather than stuck: pinned to one CPU beside two busy processes, its stack, dumped every 15 s, was still in imports after 30 s and had moved on between dumps. The failing run was the only one sharing the box with other work of mine in its first minute. It exposed a host-side flaw, fixed here: a template late to report ready was marked broken for the rest of the host's life, with an empty reason in its one warning.

  • The tests added for the review findings, each watched failing before its fix: a template late to report ready is tried again after a ten-minute rest, launches during the rest go the ordinary way quietly, launches waiting behind the late start do not each start one, and the warning names the deadline; the relay's spec carries every argument the provider's construction takes, and a tls_verify opt-out reaches the provider the host builds; a relayed agent recovers from a prompt-cache refusal with the agent and the host in separate processes, chat first, and a bare chat learns the refusal; an idle check on another loop leaves an agent's lock usable here; an agent with its own OPENAI_BASE_URL is launched without the relay while a tool's API key leaves the relay on; a Raven agent holding an armed wake is kept until the wake fires, end to end through the pool with a real stub process, and kept or closed on its _raven/residentWork answer; a relayed call is sized under the agent's turn window, and a relay started inside a host turn shows none of it to the call; a peer without the token is dropped before a frame is read (silent, and announcing a 256 MiB frame); building an agent's provider leaves the host's environment alone; a non-mapping subagents.runtime keeps the last settings; an idle check on another loop leaves this loop's connections; the bare-host test passes under a 100-character basetemp.

  • Prompt-cache refusal reproduced across two processes (a real RelayServer and LiteLLMProvider, acompletion refusing any payload that carries cache_control): at 48bb4b004 a relayed non-streamed call went out marked -> marked and failed with a 400 while a direct one recovered; at this head the relayed call goes out marked -> unmarked and succeeds, like the direct one.

  • Relayed max_tokens measured across two processes, with a real RelayServer and LiteLLMProvider and acompletion replaced by a recorder, the agent under a turn window of 32768: 32768 relayed against 16384 direct at 81b3aba66, 16384 against 16384 since 48bb4b004, whether the server was started outside a turn or inside a host turn.

  • ruff check, ruff format --check, ty check over the make lint-types targets, lint-imports (10 contracts kept) and deptry raven: clean. The module import-cycle count stays at 44, as on main.

  • npx commitlint --from origin/main --to HEAD, scripts/check_commit_messages.py origin/main..HEAD, scripts/check_large_files.py, scripts/check_source_language.py: all pass.

  • Mutation checks, each turning its own tests red: for the late template, a missed deadline marked broken, no rest after it, no second look under the lock, a warning on every launch during the rest, and a warning without its reason; for the round before, a relay spec without tls_verify; a bare chat that does not learn the refusal; one agent lock shared by every loop; never skipping the relay for an agent's own LLM environment, or counting a tool's API key as one. Earlier rounds: the pool never asking for resident work, the client ignoring the declaration, the agent not declaring it, counting another partition's cron jobs or a stopped scheduler's; the agent sending no window, the host ignoring it, relay handlers left in the start-time context; the server reading a frame before the token; building the host's provider with the environment written; the idle check without its loop filter; removing the per-event-loop rebuild of the template and of the relay; removing the loadSession condition on idle close; removing recorded_by_caller on the host; removing the template's hold task, the wait after the group kill, or the finally around the template's cleanup; removing either running-loop check; removing the no-Unix-socket guard from either connect path, or choosing TCP by platform name again; adding an importlib.import_module call under raven/acp_client; taking the template's route in the entry before the certificate-store switch.

  • Windows: the owner's native Windows 11 re-reviews ran tests/test_provider_relay.py tests/test_acp_pool_idle_close.py tests/test_acp_methods.py: 231 passed, 1 skipped at 48bb4b004, and 237 passed, 1 skipped at 2f65eee95, the head before the rebase onto b05267b2e (the skip is the Unix-only bare-host test), with the token-first, window-sizing, cross-loop, resident-work and tls_verify relay tests running. The full suite has not been run on Windows.

  • End to end at this head: five product agents through the real pool, template and relay, against a stub OpenAI-compatible endpoint. In each of two runs all five forked and completed a turn, none was launched without the relay, and asked for resident work Raven-Oncall answered resident (one running service, its oncall-flow watcher) and the other four did not.

  • Relevant tests pass locally

  • Relevant lint / type checks pass locally

  • User-facing docs or screenshots are updated when needed

Risk

  • Behaviour change: with the defaults, product agents launched on Linux run forked from a template and make their LiteLLM calls through the host, and an idle product agent's process is closed after 600 s and relaunched on its next turn, unless it holds an armed cron job or a running plugin service. That costs a cold start of a few seconds. A relayed call uses the host's environment: an agent's own env entries that LiteLLM reads turn the relay off for that agent, but differences between the login shell and the host process are not detected.

  • Security: the relay listens only in a fresh 0700 directory (loopback TCP plus a token on Windows) and serves only its token. The agent's provider construction, API key included, crosses that local socket. A connection presents the token before the host reads anything else, under a five-second deadline. The host builds an agent's provider without writing the agent's key into its own environment, so neither the host's own providers nor the processes it starts see it. Products inherit the host's LLM settings by default.

  • Known limits, left as they are: resident work is counted per partition, so while one instance of a product holds an armed cron job, every pooled instance of that product is kept; that holds memory longer, it does not lose a wake. The pool's acquire still hands a connection to whichever loop asks, without checking which loop made it, as on main; and now that each loop has its own lock, two loops launching the same agent at the same moment each start a process, and the pool keeps one. Tracked in fix(acp_client): pooled acquire ignores which event loop owns a connection #1152.

  • Rollback: set subagents.runtime to {"idleCloseS": 0, "llmRelay": false, "forkTemplate": false} for today's behaviour without a restart, or revert this commit.

  • Security impact considered

  • Backward compatibility considered

  • Rollback path is clear for risky changes

Related Issues

N/A

@LivXue
LivXue requested review from 0xKT and gloryfromca October 27, 2026 09:07

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

Blocking: fork-template cleanup and its forbidden cross-layer preload dependencies must be corrected.

I reviewed the full github/main...HEAD diff and the surrounding ACP lifecycle, provider factory/retry/streaming callers, configuration admission, protocol/session-resume behavior, and relevant history. I also checked backward-compatible direct-provider fallback, test changes, AGENTS.md, CONTEXT-MAP.md/CONTEXT.md terminology and layer constraints, and repository import contracts. I found two blockers inline.

Verification:

  • uv run pytest tests/test_acp_fork_template.py tests/test_acp_pool_idle_close.py tests/test_provider_relay.py tests/test_provider_auth_method.py tests/test_provider_catalog.py tests/test_provider_prompt_cache.py tests/test_provider_resolution_invariants.py -x: 691 passed, 104 skipped, with one fork-template subprocess-transport warning.
  • uv run pytest tests/test_providers_factory.py tests/test_per_model_provider.py tests/test_provider_protocol.py tests/test_litellm_provider_attribution.py tests/test_litellm_provider_model_names.py tests/test_litellm_provider_model_overrides.py tests/test_litellm_provider_reasoning.py tests/test_litellm_provider_stream.py tests/test_litellm_provider_stream_end.py tests/test_litellm_provider_timeout.py tests/test_provider_stream_fallback.py -x: 575 passed.
  • uv run lint-imports: 10 contracts kept, but the dynamic imports called out inline are invisible to this static check.
  • Isolated next-loop reproduction: the named fork-template test passes but prints Loop ... is closed; the larger targeted run records PytestUnraisableExceptionWarning: RuntimeError: Event loop is closed.

Comment thread raven/acp_client/fork_template.py Resolved
Comment thread raven/acp_client/fork_template.py Resolved
@LivXue

LivXue commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility review — PR #1127 @ 090a5cdec05dc2cea602001e6c47447720adc411

Scope: full base 26352da0 → this head (29 files, +4335/−345): the ACP idle-close, LLM relay (relay.py, relay_server.py, relay_wire.py, relay_provider.py), the Linux fork template (fork_template.py), provider factory/shape changes, config schema/live, and the tests.

One Windows finding (medium), plus what already works.

The relay's Unix-socket paths are unguarded on Windows, and the relay tests assume Unix sockets

Files: raven/providers/relay.py — connect() (:168-180) and _connect_sync() (:145-160); tests/test_provider_relay.py:185 (fixture params=["unix", "tcp"]), :275, :309.

Mechanism: Windows's asyncio has no open_unix_connection / start_unix_server attributes. (a) The test fixture builds a RelayServer(tcp=False) for the [unix] half, so all 13 [unix] cases error at setup with AttributeError: module 'asyncio' has no attribute 'start_unix_server'. (b) Three tests set a unix: RELAY_ENV and expect the documented fallback ("an unreachable host leaves the call to this process"), but connect() calls asyncio.open_unix_connection directly; the resulting AttributeError is not caught by except (OSError, asyncio.TimeoutError), so the call fails instead of running locally. That also means the fallback contract is untested and broken for a stale/foreign unix: address on Windows.

Evidence (native Windows 11, CPython 3.12.13):

pytest tests/test_acp_pool_idle_close.py tests/test_provider_relay.py \
       tests/test_acp_fork_template.py tests/test_provider_auth_method.py \
       tests/test_provider_catalog.py tests/test_provider_prompt_cache.py \
       tests/test_provider_resolution_invariants.py -q
→ 7 failed, 636 passed, 139 skipped, 13 errors

13 ERRORs: tests/test_provider_relay.py::*[unix]
   AttributeError: module 'asyncio' has no attribute 'start_unix_server'
3 FAILEDs: test_an_unreachable_host_leaves_the_call_to_this_process
           test_a_stream_without_a_reachable_host_runs_here
           test_a_stream_the_host_ends_oddly_or_drops_is_a_failure_here
   AttributeError: module 'asyncio' has no attribute 'open_unix_connection'

The remaining 4 failures (test_provider_prompt_cache.py::test_every_module_that_writes_the_field_asks_whether_it_may, three test_provider_resolution_invariants.py scans) fail identically on main 26352da0 (re-run there: 4 failed) — inherited, not this PR.

Remedy direction: treat "no Unix-socket API on this host" as RelayUnavailableError in relay.connect()/_connect_sync() (a hasattr(asyncio, "open_unix_connection") / hasattr(socket, "AF_UNIX") guard), so the existing local fallback applies; and make the relay test fixture pick the transport by platform (or skipif(sys.platform == "win32", ...) the [unix] half and the unix:-address cases), so Windows CI/dev signal is meaningful.

What already works on Windows: RelayServer chooses loopback TCP on win32 (relay_server.py:70-94, token-authenticated), and the fork template is explicitly Linux-gated — template_available() returns False (fork_template.py:444) and the fork tests skip correctly here (part of the 139 skips). The idle-close logic and its tests pass; no new subprocess/signal primitive is added on the Windows path.

Verification: the command above; base comparison for the inherited four as noted. Limitations: no live host/agent pair was exercised end to end (the TCP relay path is covered by the [tcp] parameterizations only), and the full suite was not run.

@LivXue
LivXue force-pushed the perf/share_subagent_child_memory branch from 090a5cd to 81b3aba Compare October 27, 2026 14:11
@LivXue

LivXue commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

Windows finding (the relay's Unix-socket paths): fixed in 73dea78, in the direction the review gave.

  • relay.connect() and _connect_sync() now treat a unix: offer on a system without asyncio.open_unix_connection or socket.AF_UNIX as RelayUnavailableError(connecting=True) before touching either API, so the documented local fallback applies and the process stops asking a host it cannot reach.
  • RelayServer picks loopback TCP by the same capability (no asyncio.start_unix_server) instead of by sys.platform == "win32".
  • The relay fixture runs its [unix] half only where asyncio.start_unix_server exists, and the test that plays the host with a bare Unix server is skipped without it.
  • test_a_unix_address_on_a_system_without_unix_sockets_runs_here deletes those attributes and checks that both a catalogue lookup and a RelayProvider.chat run in the process; removing the guard from either connect path turns it red. test_a_host_without_unix_sockets_serves_on_loopback_tcp checks the server's choice; going back to the platform-name test turns it red.

Not verified on native Windows. Simulated on Linux by running tests/test_provider_relay.py with asyncio.open_unix_connection and asyncio.start_unix_server deleted before collection: 43 passed, 1 skipped, no [unix] case collected, and test_an_unreachable_host_leaves_the_call_to_this_process and test_a_stream_without_a_reachable_host_runs_here pass through the new guard. A re-run of the review's command on Windows would confirm it.

@LivXue
LivXue requested a review from gloryfromca October 27, 2026 14:12
@LivXue

LivXue commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review — PR #1127 @ 81b3aba662ea8534b1b8c73c2af323332acb9948

Scope: the head moved from the previously reviewed 090a5cde. The delta addresses the finding I reported there: raven/providers/relay.py gains _unreachable_kind(), called by both _connect_sync and connect, so a unix: offer on a system without socket.AF_UNIX/asyncio.open_unix_connection raises RelayUnavailableError (and the documented local fallback applies) instead of an AttributeError; RelayServer now chooses loopback TCP by capability (not hasattr(asyncio, "start_unix_server")) rather than by sys.platform; and the test fixture parameterizes its transports by the same capability, with the Unix-only cases skipped on Windows. A cross-thread/loop relay reuse path was added too.

Result: the finding is fixed; no new Windows issue.

Verification (native Windows 11, CPython 3.12.13, isolated worktree D:\tmp\rv-1127-n at this head, scratch HOME/USERPROFILE, PYTHONUTF8=1):

pytest tests/test_provider_relay.py tests/test_acp_pool_idle_close.py tests/test_acp_fork_template.py -q
→ 56 passed, 27 skipped in 17.85s

The skips are the correct ones: the Unix-socket-only host test and the Linux fork-template cases. Previously this run produced 13 errors (asyncio.start_unix_server missing) and 3 failures (asyncio.open_unix_connection missing); none remain, and the [tcp] transport — the one a Windows host uses — is fully exercised.

Limitations: no live host/agent pair end to end; the full suite was not run.

@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 changes since 090a5cdec05d, including the fixes and tests for both findings I opened, the Windows relay fallback, the cross-event-loop lifecycle follow-up, the relevant callers and history, backward-compatible fallback behavior, and the updated CONTEXT.md layer boundaries. I also checked that the template-side tests moved with the code rather than being weakened. Both of my threads are now resolved.

Verification:

  • uv run pytest tests/test_acp_fork_template.py tests/test_cli_fork_template.py tests/test_acp_pool_idle_close.py tests/test_provider_relay.py -W error::pytest.PytestUnraisableExceptionWarning -x: 110 passed.
  • uv run pytest tests/test_acp_fork_template.py tests/test_cli_fork_template.py tests/test_acp_pool_idle_close.py tests/test_provider_relay.py tests/test_provider_auth_method.py tests/test_provider_catalog.py tests/test_provider_prompt_cache.py tests/test_provider_resolution_invariants.py -x: 700 passed, 104 skipped.
  • uv run lint-imports: 10 contracts kept.
  • uv run ty check raven/acp_client/fork_template.py raven/cli/_fork_template.py raven/providers/relay.py raven/providers/relay_server.py: all checks passed.

Comment thread raven/acp_client/pool.py Resolved
Comment thread raven/providers/relay_server.py Resolved
@0xKT

0xKT commented Oct 27, 2026

Copy link
Copy Markdown
Member

Not a blocker - non-blocking items from acceptance on 81b3aba6. Each was reproduced or read in full on the merged tree; none changes the grade on its own.

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. Relay admits before auth. RelayServer._serve (relay_server.py:156-163) reads a whole frame (relay_wire.read_frame, up to MAX_FRAME_BYTES = 512 MiB, no deadline, json.loads on the loop) before hmac.compare_digest. On TCP (the default where asyncio has no start_unix_server, i.e. Windows) the token is the only lock, so another local account can hold connections open and make the host buffer large frames without the token. Measured with tcp=True: a header-only peer stays open past 8s; 5 unauthenticated peers grew the host from about 30 to 187 MiB, all 5 held. The existing owner raven/mcp/transport.py reads len(token)+1 bytes under AUTH_TIMEOUT_S = 5.0 first. The relay is also a third site deciding Unix-socket availability (mcp/transport.py:15, relay_server.py:72, relay.py:148-159).
  2. Agent credentials land in the host environment. build_litellm_provider (relay_server.py:54-63) constructs LiteLLMProvider(**spec) in the host, and _setup_env (litellm_shape.py:111-138) then writes the agent's key into os.environ (overwrite for gateways, setdefault otherwise). Measured: host OPENROUTER_API_KEY went from the host's value to the agent's after one relayed call, and children the host spawns later inherit it. The host's own configured calls pass an explicit key, so this is environment pollution, not cross-billing of those calls.
  3. subagents.runtime non-mapping edit resets to defaults. subagent_runtime uses present = isinstance(section, dict) (live.py:584). So "runtime": false counts as the section leaving the file. _admit then forgets the last answer, and the defaults (relay on, fork template on, idle close 600) come back, against the reader's own docstring. Sibling readers (cron_default_timezone) treat a present non-dict as present-and-rejected.
  4. Idle task on a second loop closes the first loop's agents. _watch_idle (pool.py:696-702) starts a task on any other running loop that acquires, for example raven agent -m run via cli.dispatch on a worker thread. close_idle then takes every name's lock and closes connections owned by the other loop. Measured effects:
    • agents SIGKILLed (rc -9 instead of 0), about 2s stall per close;
    • a name lock can be bound to the worker loop, so later main-loop acquires of that agent raise RuntimeError ... bound to a different event loop until close_pool.
      Reachability needs a timing coincidence, which is why this is not filed as blocking. Scoping the idle task to connections created on its own loop would close it.
  5. macOS test failure. tests/test_provider_relay.py::test_a_stream_the_host_ends_oddly_or_drops_is_a_failure_here binds a Unix socket under tmp_path and fails in this macOS run with OSError: AF_UNIX path too long (macOS caps socket paths at 104 bytes; pytest tmp_path under TMPDIR is usually longer). The other 659 tests in the changed files pass here. A short tempfile.mkdtemp(dir="/tmp") path, as RelayServer.start itself uses, avoids it.

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

Blocking: idle close must preserve resident scheduled work, and relayed streams must size output under the agent's binding.

The two new blocking reports change my prior clean stance. I confirmed the relevant code paths: _closable treats a quiet load-capable ACP process as disposable without accounting for its in-process cron and resident services, while relayed streaming forwards the not-given max_tokens sentinel and resolves it inside the host server context rather than the agent turn context. The existing focused tests pass but do not cover either cross-boundary case.

Verification: uv run pytest tests/test_acp_pool_idle_close.py tests/test_provider_relay.py tests/test_core_cron_stack.py tests/test_cron_service_wake.py -x — 118 passed. I am not duplicating the other reviewer's findings or re-grading their threads; this comment only updates my stance on the whole record.

@LivXue

LivXue commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

Thanks for the acceptance pass. All five non-blocking items are fixed in this push:

  • N1, the frame read before the token check: dfe9fa3. A client sends the token first, alone on a line; the server reads exactly that many bytes under a 5 s deadline before any frame, drops a silent peer, and refuses a wrong token without reading more. test_a_peer_without_the_token_is_dropped_before_the_host_reads_a_frame (unix and tcp) holds a silent peer and one announcing a 256 MiB frame; at 81b3aba the host waited on both.
  • N2, the agent's key in the host environment: b373080. LiteLLMShape takes export_env, and the host's build_litellm_provider passes False; the agent's own process still exports as before. Every call passes the key explicitly, and with the variables unset LiteLLM's chat path sent it for zai, fireworks_ai, huggingface and openrouter (checked against a local endpoint). test_building_an_agents_provider_leaves_the_hosts_environment_alone covers the gateway overwrite and the env_extras alias.
  • N3, a non-mapping subagents.runtime: 50bcfae. Only a missing section or a null brings the defaults back; false, a string, a list or a non-mapping subagents keeps the last admitted settings.
  • N4, the idle check from another loop: 76bff25. Each connection records the loop it was made on, each loop runs its own idle task, and close_idle takes the locks of and closes only its own loop's connections. test_an_idle_check_on_another_loop_leaves_this_loops_connections runs a check on a worker thread's loop; at 81b3aba it closed the main loop's agent.
  • N5, the macOS socket path: 7662327. The bare-host test binds in a short tempfile.mkdtemp() directory. It passes here under a 100-character basetemp, where it failed with AF_UNIX path too long before; not run on macOS itself.

@LivXue
LivXue requested review from 0xKT and gloryfromca October 28, 2026 06:11

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

Reviewed the delta from 81b3aba662ea through this head. The resident-work extension now keeps Raven ACP agents while their partition owns armed cron jobs or started plugin services, and the end-to-end test demonstrates the wake survives idle close and the process becomes closable after it fires. Relayed chat and streaming calls now carry the agent turn window, run under that binding in a fresh handler context, and no longer inherit the host turn. The additional acceptance fixes preserve invalid live-config edits, keep agent credentials out of the host environment, authenticate before reading a relay frame, avoid overlong macOS test socket paths, and confine idle closing to the connection's event loop.

Coverage this round: repository rules and CONTEXT terms, the complete post-81b3aba662ea diff, affected callers and commit history, backward compatibility for third-party ACP agents and relay fallback, test changes for weakening, and import-layer architecture. I found no weakened tests or new merge-blocking behavior.

Verification: uv run pytest tests/test_acp_pool_idle_close.py tests/test_acp_methods.py tests/test_provider_relay.py tests/test_core_cron_stack.py tests/test_cron_service_wake.py tests/test_config_live.py -x (405 passed); uv run lint-imports (10 contracts kept); focused uv run ty check ... (passed); git diff --check 81b3aba662ea..HEAD (passed).

@LivXue

LivXue commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review — PR #1127 @ 48bb4b0041344fd3fadc695136f0ac5cbd90affa

Scope: the delta since the previously reviewed head 81b3aba6 — 7 commits, 22 files (+575/−66): token-first relay admission, a per-connection context plus the turn's window on the relay host, no environment export when the host builds an agent's provider, per-loop idle-close bookkeeping, the _raven/residentWork ask before an idle agent is closed, and retention of the last valid subagents.runtime on an invalid edit. Several of these are Windows-facing (the relay's loopback TCP transport and its admission order, asyncio loop ownership of agent processes), so I exercised them natively.

Result: no Windows-specific defect found.

  • The new greeting (token + "\n", binary bytes on the socket) and _admit are transport-agnostic; on Windows the relay is loopback TCP (RelayServer._tcp = not hasattr(asyncio, "start_unix_server")), and the Windows runs below exercised exactly that path, including the two peers that must be dropped before a frame is read. No line-ending translation or filesystem path is involved.
  • export_env=False only stops mutating the host's environment; credentials still reach every call as an explicit api_key/api_base kwarg (raven/providers/litellm_provider.py:342-348), so no Windows-specific credential path changes.
  • The per-loop idle bookkeeping (_idle_tasks keyed by loop, _Connection.loop, call_soon_threadsafe for another thread's loop) addresses closing a connection from a loop other than the one that owns its pipes; the new two-loop test ran on Windows, where those pipes are Proactor transports.
  • subagents.runtime retention and the resident-work cron/service read are platform-neutral.

Verification on native Windows 11, detached worktree D:\tmp\rv-1127 at the reviewed head, offline venv (uv sync --frozen --offline --all-extras --no-extra segmenter --dev), scratch HOME/USERPROFILE, PYTHONUTF8=1, PYTHONPATH pinned to the worktree:

  • uv run --no-sync pytest tests/test_provider_relay.py tests/test_acp_pool_idle_close.py tests/test_acp_methods.py -q → 231 passed, 1 skipped (the skip is the Unix-socket-only bare-host test, skipped by design on Windows). No failures, so no base comparison was needed.
  • Targeted confirmations that the new paths ran rather than skipped: the token-drop, window-sizing and host-context tests → 3 passed; the resident-work and cross-loop tests → 2 passed; test_an_agent_holding_a_wake_is_kept_until_the_wake_has_fired (real stub process, arm-and-fire) → 1 passed.

Coverage gaps: the full suite was not rerun for this delta; Unix-socket transport behaviour cannot be exercised on this host; the relay tests use the scripted _Wire provider, so no live vendor call was made; an in-place upgrade that leaves an old agent process speaking the pre-greeting protocol to a new host was not tested here.

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.

Blocking: under the relay, a non-streamed call cannot recover from an upstream that refuses cache_control. A direct agent recovers. The suppression is learned in the agent's process, but the request is built in the host's.

Mechanism.

  • chat_with_retry handles a refuses_prompt_cache verdict by calling prompt_cache.suppress(model), stripping the marks and resending (raven/providers/base.py:796-800).
  • prompt_cache._SUPPRESSED is process-local (prompt_cache.py:187). Under the relay the agent's RelayProvider does that suppress in the agent process.
  • The resend is built on the host. relay_server._chat calls the host's LiteLLMProvider.chat, whose gate _supports_cache_control reads the host's own _SUPPRESSED and puts its marks back on the stripped retry (litellm_provider.py:312-318). strip() also drops the marks-placed claim, so loop-claimed requests are re-marked too.
  • The retry is refused again, the one dropped_cache_control is spent, a 400 is not retryable, and the call fails.
  • LiteLLMProvider.chat never calls suppress (only chat_stream does, :499). So the host learns only if a stream happens to hit the same model first.

Reproduced (lane probe, real processes on both sides: agent RelayProvider against a real RelayServer with its real LiteLLMProvider; only acompletion faked to refuse any payload carrying cache_control, as OpenRouter does):

direct (no offer)  : CHAT finish=stop content=ok    wire: marked -> unmarked
relay, chat first  : CHAT finish=error (BadRequestError ... cache_control is not permitted)
                     wire on the host: marked -> marked   (host re-marked the stripped retry)
relay, stream first: ok   (the stream taught the host)
base tip control   : CHAT finish=stop content=ok

Production non-streamed callers include the curator (context_engine/segments/curator.py:277), the permission judge (permissions/judge.py:145), the consolidator, the heartbeat, an unstreamed turn (agent/harness/action.py:87), and #1092's unstreamed resend (turn_path.py:770/819).

Rule. This PR's own SubagentRuntimeConfig docstring (raven/config/schema.py:2453-2458): "None of it changes what an agent does". And llm_relay: "The agent builds the request, retries and records usage itself".

Fix direction: carry the agent's suppression with the call. For example, send the agent's suppressed set (or a per-call "no cache marks" flag) in the relayed request and have the host build under it. Or have the host's non-streamed path learn from the refusal the way chat_stream does. Add a cross-process test for the chat-first order.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 2f97829.

  • LiteLLMProvider.chat now suppresses the model when it classifies a refusal of cache_control, as chat_stream already did. Behind the relay the host learns from the first refusal, so the agent's stripped resend goes out unmarked: the same two requests a direct agent makes. The retry stays in the agent's chat_with_retry; the host only learns.
  • Reproduced first on 48bb4b0 with two processes (host: a real RelayServer and LiteLLMProvider, acompletion refusing any payload that carries cache_control; agent: RelayProvider.chat_with_retry in its own process): direct marked -> unmarked, ok; relayed chat first marked -> marked, 400; relayed stream first, ok. On this head the relayed chat-first case is marked -> unmarked, ok.
  • Tests, both red before the fix: test_an_agent_recovers_from_a_cache_refusal_through_a_host_in_another_process runs the agent and the host as separate processes, chat first (the in-process fixtures share one suppression set, as the second review noted), and test_a_bare_chat_learns_a_refusal_as_a_stream_does checks that a bare chat learns the refusal and that an unrelated 400 is not taken for one.

@0xKT

0xKT commented Oct 29, 2026

Copy link
Copy Markdown
Member

Not a blocker - delta acceptance on 48bb4b00 (onto ea4cf3bf). The two earlier blocking threads stay fixed: resident work keeps an agent with armed cron work, and a relayed call is sized under the agent's window. The five earlier non-blocking items are addressed: token read first, no key export on the host, the non-mapping runtime section, idle close per loop, and the macOS socket path.

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.

Non-blocking items from this pass:

  1. A worker loop's idle check can still bind an agent's name lock. close_idle takes self._lock_for(name) whenever any binding of that name has a connection on the running loop (raven/acp_client/pool.py:720-721). The per-name asyncio.Lock is shared across loops (:412-421) and binds to the loop of its first waiter. So a worker loop (e.g. raven agent -m via cli.dispatch) that holds one binding of an agent, while the main loop holds another binding of the same agent, can bind the lock to itself while main holds it. After that, a contended main-loop acquire of that agent raises RuntimeError ... bound to a different event loop for the life of the process. Measured: 3/3 with only the worker's check running, 1/11 with both checks as wired; control without the worker's check 11/11 clean. The repo's own convention for this primitive is a per-loop lock table (raven/agent/subagent/dag_store.py:22-26).
  2. _raven/residentWork counts the whole acp partition. armed_jobs returns every enabled job whose channel the service owns (cron/service.py:690-698). Every raven acp serves channel acp against the one jobs.json. So one recurring acp reminder anywhere on the home, or any channel-less legacy job, makes every pooled raven agent answer resident: true, and idle close never closes any of them. Measured with two real CronServices on one store: after a sibling armed a job, the handler answered {'resident': True, 'cronJobs': 1}. Counting jobs this process armed (or jobs whose session this agent holds) would keep the idea and its benefit.
  3. A relayed call reads LiteLLM's env-held settings from the host. The spec carries constructor arguments only, so anything LiteLLM reads from os.environ at call time comes from the host: a direct vendor's base URL, ambient credentials (bedrock AWS_*), proxy/CA. The agent's launch env (login shell plus the per-agent env map) no longer applies to its own calls. Measured with real LiteLLM: an agent env OPENAI_BASE_URL reaches the agent's endpoint locally, but goes to the default endpoint when relayed. Worth stating, or carrying the agent's relevant env with the spec.

Tests here on the merged tree: the 9 changed Python test files via the board runner gave 839 passed and 144 skipped.

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

Blocking: relayed non-streamed calls must preserve prompt-cache suppression across the agent and host processes.

The new blocking report changes my prior clean stance. I confirmed the mechanism in the current code: chat_with_retry() records a cache-control refusal in the agent process and strips the retry payload, but RelayServer._chat() rebuilds that request through the host process's LiteLLMProvider.chat(). Because the host has a separate suppression set, it re-adds cache marks and the one downgrade retry is refused again. This affects ordinary non-streamed relay callers and differs from direct execution.

Verification: uv run pytest tests/test_provider_relay.py tests/test_provider_prompt_cache.py -x — 129 passed. Those tests do not cover the production boundary: the in-process relay fixtures share prompt_cache._SUPPRESSED, while the reported chat-first reproduction uses separate agent and host processes. I am not duplicating or re-grading the other reviewer's finding; this comment updates my stance on the whole record.

@LivXue
LivXue force-pushed the perf/share_subagent_child_memory branch from 48bb4b0 to 2f65eee Compare October 29, 2026 12:37
@LivXue

LivXue commented Oct 29, 2026

Copy link
Copy Markdown
Member Author

Thanks. On the three non-blocking items:

  • N1, the agent lock bound by a worker loop: fixed in 87926ab. Locks are kept per running loop, as dag_store keeps its own. test_an_idle_check_on_another_loop_leaves_an_agents_lock_usable_here reproduces the binding (3 of 3 runs raised bound to a different event loop at 48bb4b0) and passes now. One thing this does not cover, and which predates this PR: acquire is not loop-aware either. A worker loop asking for a binding the main loop holds is handed the main loop's connection, and with per-loop locks two loops launching the same binding at once are no longer serialized. Keeping each loop's connections apart in acquire is a larger change to the pool; I would take it as a follow-up.
  • N2, resident work counted over the whole acp partition: kept as is, deliberately. The store is per product home (get_runtime_subdir("cron") under the home each product renders), so the scope is one product's instances rather than every pooled agent. Within that scope any process whose scheduler runs may claim a due job, so counting only the jobs one process armed could let it be closed while no other instance is left to fire them. Erring towards keeping a process costs memory, never a wake; a finer count (by the session a job targets) can come later.
  • N3, the host's LiteLLM environment: fixed in 6f9eead. An agent whose own configured env sets something LiteLLM reads at call time beside the explicit key and base (other endpoints and API versions, AWS_*, AZURE_*, Vertex credentials, proxies, CA bundles) is launched without the relay and makes its calls itself, so an OPENAI_BASE_URL in an agent's env reaches that agent's endpoint again. Only the agent's own map is compared with the host: on my machine the login shell and the host already differ in NO_PROXY casing, which would have turned the relay off for every agent. API keys are left out, since every call passes its key explicitly. test_an_agent_configured_with_its_own_llm_environment_makes_its_own_calls covers both sides: an OPENAI_BASE_URL turns the relay off, a tool's SERPER_API_KEY does not. The login shell's own differences from the host stay a stated limitation in the description.

@LivXue
LivXue requested review from 0xKT and gloryfromca October 29, 2026 12:37

@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; suggestions only, and they are marked inline.

Reviewed the rebased revision by range-diffing it against the previously reviewed series and reading the four new commits plus their callers and tests. The cache-control blocker is fixed: the host's non-streamed provider now learns the refusal, and the two-process regression test proves the relayed retry changes from marked to unmarked. The new per-loop lock table, agent-specific LLM-environment fallback, and tls_verify relay construction are consistent with the current main-line provider and pool changes.

Named follow-up from the record: pooled acquire remains unaware of event-loop ownership, so a worker loop can reuse a connection owned by another loop or race a same-binding launch. The author identifies this as pre-existing and has explicitly deferred the larger pool separation change; it is nonblocking here.

Coverage this round: repository rules and CONTEXT terms, the full prior series through range-diff plus the post-review delta, affected callers and history, backward compatibility and fallback behavior, tests for weakening, and import-layer architecture. No new plain error was found.

Verification: uv run pytest tests/test_provider_prompt_cache.py tests/test_provider_relay.py tests/test_provider_tls_verify.py tests/test_acp_fork_template.py tests/test_acp_pool_idle_close.py -x (203 passed, with two LiteLLM logging coroutine warnings); uv run lint-imports (10 contracts kept); focused uv run ty check ... (passed); git diff --check github/main...HEAD (passed).

@LivXue

LivXue commented Oct 29, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review — PR #1127 @ 2f65eee953d9dbd9d8e439f0d6b0a28d9baa52e6

Scope: the branch is rebased onto current main and extended; git range-diff 855a412fa..48bb4b00 3ba6179f..2f65eee9 shows the commits otherwise content-identical to the previously reviewed head. The two changes since then are: a new commit, 2f65eee9 ("carry tls_verify across the relay"), which adds tls_verify to the relay's provider spec so a relayed agent's call is not verified on the host after the agent's own section opted out; and the rebase adaptation to main's litellm_shape.py split, which keeps the same field there. Re-ran the affected suites on Windows.

Result: no Windows-specific defect introduced by this head.

Verification on native Windows 11, detached worktree D:\tmp\rv-1127b at the reviewed head, offline venv (uv sync --frozen --offline --all-extras --no-extra segmenter --dev), scratch HOME/USERPROFILE, PYTHONUTF8=1, PYTHONPATH pinned:

  • uv run --no-sync pytest tests/test_provider_relay.py tests/test_acp_pool_idle_close.py tests/test_acp_methods.py -q → 237 passed, 1 skipped (the skip is the Unix-socket-only transport test). No failures, so no base comparison was needed.

Coverage gaps: the full suite was not rerun; the Unix transport cannot be exercised on this host; no live vendor call was made.

@LivXue

LivXue commented Oct 30, 2026

Copy link
Copy Markdown
Member Author

@gloryfromca Thanks for the re-review. Your review says the suggestions are marked inline, but none came through: the review has no inline comments attached (pulls/1127/reviews/5697843319/comments is empty). If you wrote some, they may still be sitting in a pending review on your side. Could you post them, or confirm there are none?

The pooled acquire follow-up you recorded is now #1152, with a reproduction: two event loops on two threads acquiring the same agent at the same moment. At this head each starts its own process and the pool keeps only one, so the other runs outside its bookkeeping. On main the same run hangs instead, because the one shared lock is released from the other thread. This PR leaves acquire as it is; the description's Risk section now says so.

@gloryfromca

Copy link
Copy Markdown
Member

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

@LivXue Confirmed: there were no inline comments and no pending review comments to post. The fixed nonblocking first-line wording was misleading in this case; the sole nonblocking item was the pooled acquire event-loop limitation named in the review summary, and #1152 now tracks it with the reproduction. Nothing is missing from this PR review.

Verification: uv run pytest tests/test_acp_pool_idle_close.py -x — 21 passed; pytest emitted one warning that the real wake test spent 3.65 s idle and could be marked as a slow/production-timing test.

LivXue added 19 commits October 31, 2026 17:01
…other

Each product agent under agents/ ran as a whole Raven process of its
own, about 240 MiB PSS, nearly all of it private: a host running the
five shipped products paid about 1.2 GiB for them. Three mechanisms,
each switchable live under subagents.runtime in the host's config:

- idleCloseS (default 600): the ACP pool closes an agent's process once
  nothing is in flight either way and nothing has been said for the
  budget. Only an agent that advertises loadSession, or one that never
  opened a session, is closed, so a stateful instance's next turn
  reopens its session in a fresh process. A held wake is a prompt still
  waiting for its answer, so it is never idle.

- llmRelay (default on): the host makes a Raven agent's LiteLLM calls,
  so the agent never imports LiteLLM. LiteLLMProvider's LiteLLM-free
  construction and answers move to LiteLLMShape, which RelayProvider
  shares; only the single chat / chat_stream attempt and the catalogue
  lookups that need LiteLLM's tables (the relayed decorator in rates,
  capabilities, common_models and the Anthropic adapter) cross a local
  socket with a token. Retry, classification, usage records and traces
  stay in the agent; the host's calls run under recorded_by_caller. An
  agent that cannot reach its host loads LiteLLM as before.

- forkTemplate (default on, Linux only): a template process that has
  already imported what agents import forks each Raven agent the host
  launches, so the loaded pages are shared. It preloads the modules at
  least two products import, learned per product from the agents
  themselves (one product's own while only one has been seen), plus the
  token estimator's table. A launcher's os.execv of python -m raven runs
  Raven in the same process. Anything else, and any failure, launches
  the ordinary way.

The relay and the template are per event loop, since a CLI turn is one
asyncio.run. raven acp takes the relay offer out of its environment at
startup, and the token estimator drops the duplicate copy of its ranks
that LiteLLM's import used to drop.

Measured with five product agents, one turn each (PSS): 1414 -> 724 MiB
in total, the agents 1259 -> 547 MiB. One agent alone: 402 -> 371 MiB.
ty does not honour a mypy-style type: ignore on the relayed decorator's
return, and the lint job failed on it with invalid-return-type. The
wrapper stands in for the decorated function by construction, so it is
cast to the decorated type instead of silenced.
…sockets

On Windows asyncio has no open_unix_connection and socket has no
AF_UNIX, so a unix: offer raised AttributeError past the relay's
fallback instead of leaving the call to this process. Both connect
paths now answer that case as an unreachable host, and the relay
server picks loopback TCP by the same capability rather than by the
platform name. The relay tests run the unix transport only where it
exists.
The template process loads, by name, the modules agents import -- the
CLI, the ACP server, the RPC stack, the agent loop -- and runs python
-m raven, which the acp_client shelf may not do. import-linter could
not see it because the edges were strings. That side moves to
raven/cli/_fork_template.py, a launcher, and the host starts it through
Raven's entry point (python -m raven --fork-template), which hands it
the arguments. acp_client names no module of it and imports nothing of
it; a test holds acp_client to no importlib or runpy loading.

Going through the entry also settles the certificate store before the
template imports anything. The template used to preload aiohttp, which
builds its default TLS context on import, before any switch, so a
forked agent kept a context that did not trust the system store. The
in-process test that runs python -m raven now opts out of that switch,
which had leaked truststore into the test process. The template-process
tests move to tests/test_cli_fork_template.py.
…ed it

A template was ended by close() or, once its loop had gone, by
abandon(), which killed it but neither reaped it nor closed its
transport. On a pidfd host the closed loop's watcher could no longer
reap it, so it stayed a zombie, and the transport's finalizer later
raised RuntimeError: Event loop is closed. A task on the template's own
loop now holds it, and when asyncio.run cancels that task at the end of
the run it closes the template while the loop can still reap it and
close its pipes. close() waits out the group kill as well, and a close
cut short by the loop's end still kills the template and closes its
pipes. abandon() stays as the fallback for a loop closed some other
way, and marks the transport closed so its finalizer does not run
again.
…runs

A host can run a second event loop beside its own: the RPC server runs
a CLI command such as raven agent -m on a worker thread, under its own
asyncio.run. When that loop launched a Raven agent, the launch took the
first loop's template and relay for ones an ended loop had left behind
and abandoned them, and every agent forked from that template died
with it through the parent-death signal. Only a template or relay
whose loop has stopped is abandoned now. A loop running beside them
launches its agents the ordinary way and is offered the relay's socket,
which the loop that owns it goes on answering.
The test that plays a misbehaving relay host bound its Unix socket
under tmp_path, which on macOS is already past the 104-byte cap on a
socket path, so it failed with AF_UNIX path too long. It now binds in
a short directory of its own, as the relay server itself does.
… mapping

subagent_runtime read anything but a mapping at subagents.runtime as
the section having left the file, so an edit such as "runtime": false
forgot the last valid settings and brought the defaults back: relay
on, fork template on, idle close at 600 s. Only a missing section or a
null brings the defaults back now. Any other value is a rejected edit,
and the last admitted answer stays, as the reader's docstring says.
…ng its key

The relay host built a LiteLLMProvider from the agent's construction,
and the constructor wrote the agent's key into os.environ: over the
host's own value for a gateway, beside it otherwise. The host's later
providers could pick the agent's key up, and every process the host
started after inherited it. LiteLLMShape takes export_env, and the
host passes False. Every call already passes the key explicitly, and
LiteLLM's chat path was checked to send it with the variables unset
(zai, fireworks_ai, huggingface, openrouter).
The relay server read a whole frame, up to 512 MiB and with no
deadline, before it compared the token. On loopback TCP, where any
local account can connect, a peer without the token could hold
connections open and make the host buffer large frames. A client now
sends the token first, alone on a line, and the server reads exactly
that many bytes under a five-second deadline before it reads a frame.
A silent peer is dropped, and a wrong token is refused without
anything more being read.
The pool kept one idle-check task and restarted it on whichever loop
acquired next, and close_idle closed every connection held. When the
RPC server ran a CLI command such as raven agent -m on a worker
thread, that loop's check could close the main loop's agents, killing
them after a stalled teardown and binding the pool's per-name locks to
the worker loop. Each connection now records the loop it was made on,
each loop runs its own idle check, and a check takes the locks of and
closes only its own loop's connections.
A relayed chat_stream resolved max_tokens on the host, through
send_max_tokens and the model binding active there. The relay
server's handlers ran in a copy of the context the server was started
in, so that binding was none or the host turn that first delegated,
never the agent's: an agent whose loop reserved 16384 sent 32768. The
agent now sends its turn's context window with each call, and the
server serves each connection in a fresh contextvars.Context and makes
the call under a binding with that window, so both sides size the
ceiling with the same function and the same inputs. Measured across
two processes with the real provider: 16384 relayed and 16384 direct.
CONTEXT.md's LLM relay entry also states the token-first and
environment guarantees.
Idle close judged an agent from the wire alone, so a Raven agent that
had armed a cron wake or was running a plugin service looked idle and
was closed after 600 s: the wake never fired, and the service died
with the process. Raven agents now declare raven.residentWork in
agentCapabilities._meta and answer _raven/residentWork from their own
state: the enabled cron jobs of their partition while the scheduler
runs, and the plugin services started. The pool asks only an agent
that declared it, only once closing it is otherwise settled, and keeps
it while it says yes. An agent that does not answer in time, or never
declared it, is closed as before. The default stays 600 s. Resident
work is a new term in CONTEXT.md, and the config field and the agent
integrations page say the same.
LiteLLMProvider.chat returned a cache_control refusal as an error and
left the downgrade to the retry layer, which suppresses the model in
its own process. Behind the LLM relay that layer runs in the agent,
while the resend it strips is built by the host's provider, which
still took the model to accept marks and put them back: the one
downgrade was refused again and the call failed. chat now suppresses
the model on a refusal, as chat_stream already did, so the host learns
from the first refusal and the resend goes out unmarked, the same two
requests a direct agent makes. A test runs the agent and the host as
separate processes, chat first, which in-process tests cannot show:
there the two sides share one suppression set.
The pool kept one asyncio.Lock per agent name for every loop. A lock
binds to the loop of its first waiter, so a worker loop's idle check
(a CLI command the RPC server runs on a thread) that waited on a lock
the main loop held bound it to the worker loop, and every later wait
on the main loop raised that the lock was bound to a different event
loop. Locks are now kept per running loop, as dag_store keeps its own,
and a finished loop's table goes with it.
… the relay

A relayed call is made in the host, so LiteLLM reads the host's
environment for anything the provider does not pass explicitly: other
endpoints and API versions, cloud credentials, proxies and CA bundles.
An agent whose configured env set one of these, OPENAI_BASE_URL for
instance, had it ignored once relayed. Such an agent is now launched
without the relay and makes its calls itself. Only the agent's own map
is compared with the host: the login shell differs from a host's in
ways nobody chose for one agent, and reading those would turn the
relay off for every agent. API keys are left out, since every call
passes its key. CONTEXT.md and the agent integrations page say so, and
CONTEXT.md's LLM relay entry also notes the host learning a
prompt-cache refusal.
The per-section tls_verify option is part of a provider's construction:
a section behind a self-signed gateway turns certificate checks off.
The relay's spec, from which the host builds an agent's provider,
lists the construction by hand and did not carry it, so a relayed
agent's calls were checked on the host and refused by its gateway. The
spec now carries it, and a test holds the spec to every argument the
construction takes, so an argument added there later cannot be
dropped the same way.
The test starts a real stub ACP process and waits out the one-second
wake it arms, so most of its wall clock is idle: 1.35 s to 3.65 s
depending on the machine, against the suite's 3.0 s idle ceiling,
which CI's unit shards enforce strictly. A real subprocess that cannot
be faked and still runs in the default selection is what the slow
marker is for.
A template that did not report ready within 60 s was marked broken, so
every later launch on that loop went the ordinary way, without another
word, for the life of the host: one slow start on a busy machine gave
up the memory the template shares until a restart. The one warning it
left ended in an empty reason, since a TimeoutError prints as nothing.

A missed deadline now ends that template, and launches go the ordinary
way for ten minutes, at once and without a warning each, before a new
one is started. The warning names the deadline and the retry. Launches
that waited behind a failed start check again under the lock rather
than each wait out a start of their own. A template that reports it
cannot serve, or fails to start any other way, is still not used again
by that loop.
@LivXue
LivXue force-pushed the perf/share_subagent_child_memory branch from 2f65eee to 979c031 Compare October 31, 2026 17:10
@LivXue

LivXue commented Oct 31, 2026

Copy link
Copy Markdown
Member Author

@gloryfromca Thanks for confirming. The real-wake test is now marked slow (991048b55): it starts a real stub ACP process and waits out the wake it arms, which is what the marker is for, so the strict idle ceiling in CI's unit shards no longer counts it. All 21 tests in the file still run in the default selection. With --idle-ceiling=0.5 --idle-ceiling-strict the file failed on that test before the marker (1.55 s idle of 2.00 s here) and passes after.

One more change in this push, which neither of you raised: re-running the full suite on the rebased branch, one run in six failed test_an_acp_launch_of_a_raven_agent_is_forked_and_offered_the_relay because the fork template did not report ready within 60 s. The template was slow, not stuck, but the host then marked it broken for good, so every later launch on that loop went the ordinary way without another word, and the one warning ended in an empty reason. 979c0312f ends a late template and tries a new one ten minutes later, keeps launches in between quiet, makes launches waiting behind the late start look again under the lock, and names the deadline in the warning; a template that cannot serve is still not used again. Two tests pin it, and each of the five pieces is mutation-checked.

@0xKT The branch is now at 979c0312f, rebased onto current main. The fix for your B1 thread is 1b6cbc441 after the rebase (it was 2f97829b0), with the two-process regression test; N1 and N3 are fixed, N2 is kept with its scope stated under Risk, and the pooled acquire limit is tracked in #1152. Once you have re-checked, could you resolve the B1 thread and, if it holds up, approve?

@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; suggestions only, and they are marked inline.

Reviewed the live head 979c0312f (the supplied worktree was still at 2f65eee953d9, so I fetched the PR head). Range-diff confirms the previously reviewed series is patch-identical after the rebase; the only new changes are the real-wake test's slow marker and the fork-template late-start recovery. The marker leaves the test in the default selection while excluding its intentional real-process wait from the strict idle ceiling. A template that misses readiness is now torn down, ordinary launches proceed quietly during a bounded rest, queued launches recheck under the lock, and a later launch retries with a fresh template. I found no new plain error.

Named nonblocking follow-up: #1152 still tracks the pre-existing pooled-acquire event-loop ownership limitation. There are no missing inline comments.

Coverage this round: repository rules and CONTEXT terms, range-diff/history, the complete new delta and callers, cleanup and fallback behavior, backward compatibility, tests for weakening, and import-layer architecture.

Verification: uv run pytest tests/test_acp_pool_idle_close.py --idle-ceiling=0.5 --idle-ceiling-strict -x (21 passed); uv run pytest tests/test_acp_fork_template.py -x (30 passed); uv run lint-imports (10 contracts kept); uv run ty check raven/acp_client/fork_template.py (passed); git diff --check github/main...HEAD (passed). An exploratory combined strict-ceiling run passed all 51 assertions but exited nonzero because four pre-existing fork-template tests exceed the artificial 0.5 s ceiling and are unmarked; the new pool marker behaved as intended.

@LivXue

LivXue commented Oct 31, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility re-review — PR #1127 @ 979c0312f9914cb5838035a7cf23e56c50046dd4

Scope: the branch is rebased onto main b05267b2; git range-diff against the previously reviewed head 2f65eee9 shows the 17 earlier commits identical after the rebase, plus 2 new commits: a real-wake idle-close test marked slow, and a fork template that misses its 60 s ready deadline now rests for ten minutes and is retried instead of being marked broken for the life of the host.

Windows angle: none for this delta, and by design for the changed path. The fork template is gated on template_available() — sys.platform.startswith("linux") plus os.fork/socket.send_fds — so the new rest-and-retry logic never executes on Windows; the other commit only marks an idle-close test slow. No path, argv, subprocess, encoding, or filesystem surface is added for Windows in these commits, and the rest of the PR (relay provider/server/wire, pool) is unchanged since the earlier reviews of this head's ancestors.

Verification (native Windows 11; detached worktree D:\tmp\rv-1127 at head; offline venv; scratch HOME/USERPROFILE; PYTHONUTF8=1; PYTHONPATH pinned):

  • uv run --no-sync pytest tests/test_acp_fork_template.py tests/test_acp_pool_idle_close.py -q -n 0 --tb=line → 21 passed, 30 skipped in 19.1 s. Every skip is test_acp_fork_template.py's "forking agents is Linux-only"; the idle-close suite (including the newly marked test) passes on Windows.
  • Both files are new in this PR and absent at base b05267b2, so there is no base ID set to compare; the previously reviewed commits are byte-identical per range-diff.

Coverage gaps: the fork-template rest-and-retry path cannot be exercised on this host (Linux-only), so its behaviour is verified by the Linux-skipped tests only; the relay/provider suites were not rerun because they are unchanged since the last reviewed head.

No Windows-specific issue found in this delta.

This branch has not been deployed

No deployments
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