Skip to content

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

Merged
0xKT merged 20 commits into
mainfrom
perf/share_subagent_child_memory
Nov 2, 2026
Merged

0xKT merged 20 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.

Comment thread raven/providers/relay_server.py Resolved
@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 14 commits November 2, 2026 05:44
…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 979c031 to c6f0e7e Compare November 2, 2026 05:47

@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 c6f0e7e716a8 after its rebase onto current main. git range-diff shows all 19 PR commits patch-identical to the previously reviewed 979c0312f series, so I focused on base interaction and re-ran the affected lifecycle and relay suites. I found no new plain error.

Named nonblocking follow-up: #1152 still tracks the pre-existing pooled-acquire event-loop ownership limitation. The fixed first-line wording mentions inline suggestions, but there are no inline comments in this review.

Coverage this round: repository rules and architecture contracts, range-diff and history, the PR diff and affected callers as previously reviewed, backward compatibility and fallback behavior, and test changes for weakening. Both threads I opened remain resolved.

Verification:

  • uv run pytest tests/test_acp_fork_template.py -x: 30 passed; the file emitted its existing non-strict idle-time warning.
  • 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_provider_relay.py -x: 67 passed.
  • uv run lint-imports: 10 contracts kept.
  • Focused uv run ty check ...: passed.
  • git diff --check github/main...HEAD: passed.

@LivXue

LivXue commented Nov 2, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility review — PR #1127 @ c6f0e7e716a8df3e713735f1c26770350f17b5fc

Scope: full PR diff against base 781c614a61 (current main tip), 41 files, +5546/−359: the LLM relay (raven/providers/relay*.py), the Linux fork template (raven/acp_client/fork_template.py, raven/cli/_fork_template.py), ACP idle close (raven/acp_client/pool.py, raven/acp_client/client.py), the subagents.runtime config, and their tests. The branch was rebased onto the current main; git range-diff b05267b2..979c0312f9 781c614a61..c6f0e7e71 reports all 19 PR commits content-identical to the previously reviewed head 979c0312, so this review re-verifies that content at the new (rebased) SHA on Windows.

Verdict

No Windows-specific issue found at this head.

What was checked, and how

1. Fork template is inert on Windows. fork_template.template_available() is sys.platform.startswith("linux") and hasattr(socket, "send_fds") and hasattr(os, "fork"); on both Windows interpreters tested (venv CPython 3.12.13, system 3.13.12) AF_UNIX, start_unix_server, open_unix_connection, send_fds, os.fork and SIGKILL are all absent, so fork_launch() returns None before any pipe/socket/killpg call and the ordinary launch is used. The two fork-template test modules are gated with the same predicate:

pytest tests/test_acp_fork_template.py tests/test_cli_fork_template.py tests/test_acp_pool_idle_close.py -q -n 0
-> 21 passed, 44 skipped in 14.47s   (all 44 skips: "forking agents is Linux-only")

2. The relay works through its Windows path. RelayServer._tcp is chosen when asyncio.start_unix_server is absent; it is absent on Windows, so the server binds 127.0.0.1:0 and admits peers token-first (constant-time compare, 5 s deadline) — the loopback-TCP fallback the module documents for Windows. The relay suite runs the real TCP path here:

pytest tests/test_provider_relay.py -q -n 0
-> 51 passed, 1 skipped in 14.73s   (skip: "the bare host listens on a Unix socket")

That includes test_a_host_without_unix_sockets_serves_on_loopback_tcp, test_a_unix_address_on_a_system_without_unix_sockets_runs_here, the token-first admission tests, and the two cross-process relay tests (one is the pre-existing slow-wait warning only).

3. Idle close is loop-safe on the Windows event loop. pytest tests/test_acp_pool_idle_close.py -q -n 0 -> 21 passed in 6.56s, including the per-loop lock and the real-wake/resident-work cases (the latter marked slow as intended).

4. The new launch path does not touch non-Raven agents. _share_with_raven_agent() calls raven_agent_argv() first; stub/third-party agents (copilot --acp, python stub.py) fail that check and keep their environment unchanged, and the fork branch is unreachable on Windows as above. pytest tests/test_subagent_acp.py -q -n 0 at head: 18 failed, 361 passed, 2 skipped; the same 18 test IDs fail at base 781c614a (comm diff empty in both directions) — the inherited Windows npx/file-tracking baseline, not this PR.

5. Other changed areas. pytest tests/test_provider_auth_method.py tests/test_provider_catalog.py tests/test_provider_prompt_cache.py tests/test_provider_resolution_invariants.py tests/test_acp_methods.py -q -n 0 -> 4 failed, 751 passed, 105 skipped. All 4 failures also fail identically at base: they compare source-file paths built by Path.relative_to (which uses \ on Windows) against allow-lists written with /, so the invariant scanners cannot match on Windows. Inherited test brittleness, not code this PR changed.

Coverage gaps / limitations

  • The fork-template path itself cannot be exercised on Windows — it is Linux-only by design and is covered by reading plus the skip guards, not by execution here.
  • No macOS/Linux run from this host; the review is Windows-only as scoped.
  • Full suite not rerun; browser/segmenter extras were not installed offline (neither is imported by the changed code).
  • The idle-close real-wake test runs on Windows and passed, but its timing is inherently wall-clock sensitive.

…elper

Both exits answer the prompt before any mode's own script runs, so they now share one helper. Empty prompts still settle first, and _PROMPTS is still counted in handle_prompt before either exit.

@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 sole change since c6f0e7e716a8: the ACP test stub now shares its two pre-script prompt exits through _answers_without_a_turn. The refactor preserves ordering and state: _PROMPTS is incremented before the helper, configured empty prompts still settle before resident mode, resident mode still arms its wake once applicable, and all other modes continue into their existing scripts with the same session id. I found no plain error or weakened assertion.

Named nonblocking follow-up: #1152 still tracks the pre-existing pooled-acquire event-loop ownership limitation. The fixed first-line wording mentions inline suggestions, but this review has no inline comments.

Coverage this round: repository rules, the complete one-file delta and its two callers, commit history, backward compatibility of the stub modes, tests for weakening, and architecture impact (none; test-only refactor). Both threads I opened remain resolved.

Verification:

  • uv run pytest tests/test_subagent_acp.py::test_the_note_waits_for_a_reply_when_the_fresh_sessions_turn_fails tests/test_acp_pool_idle_close.py -x: 22 passed.
  • uv run ruff check tests/acp_stub_server.py: passed.
  • uv run ty check tests/acp_stub_server.py: passed.
  • git diff --check github/main...HEAD: passed.

@LivXue

LivXue commented Nov 2, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility review — PR #1127 @ 52aec16d55e1ca4fa81ae7ad43e3143f301dfb9e

Scope: full PR diff against current main 781c614a617f (41 files, +5559/−362), and the delta since the previously reviewed head c6f0e7e716: one commit, 52aec16d5 test(acp_client): fold the stub's turn-ending prompt exits into one helper, touching only tests/acp_stub_server.py (+16/−6). That helper moves the ACP_STUB_EMPTY_PROMPTS and resident early exits into _answers_without_a_turn(), called from handle_prompt after _PROMPTS is incremented — the same order and conditions as before. The product code is unchanged since the reviewed head, whose Linux-only fork-template guard and Windows loopback-TCP relay were verified there.

Verdict

No Windows-specific issue found. The delta is test-harness code with no OS-specific calls (the stub reads os.environ mode variables it already read).

Verification (native Windows, detached worktree at the new head)

uv sync --frozen --offline --all-extras --no-extra segmenter --no-extra browser --dev
pytest tests/test_subagent_acp.py tests/test_acp_pool_idle_close.py tests/test_subagent_github_copilot.py -q -n 0
-> 19 failed, 419 passed, 2 skipped in 120.75s

The 19 failures are exactly the inherited Windows baseline seen at the previously reviewed head: the 18 test_subagent_acp.py npx/file-tracking cases (POSIX path assumptions) and test_subagent_github_copilot.py::test_an_install_the_login_path_misses_is_not_reported_as_absent (extensionless copilot fixture; shutil.which matches only PATHEXT on Windows while os.access accepts the file). The same 19 fail at base 781c614a; the pass count is identical to the head reviewed before this commit (361 + 21 + 37 = 419), so the stub refactor changes no test outcome. No new failing ID.

Coverage gaps / limitations

  • Only the stub-dependent suites were rerun for this delta; the rest of the PR is byte-identical to the reviewed c6f0e7e7 head (relay 51 passed / 1 skip, idle-close 21 passed, fork-template modules skipped as Linux-only, and the 18+1 inherited failures listed above).
  • No macOS/Linux run from this host; the review is Windows-only as scoped.

@0xKT
0xKT merged commit 945ddd7 into main Nov 2, 2026
27 checks passed
@0xKT
0xKT deleted the perf/share_subagent_child_memory branch November 2, 2026 09:44
LivXue added a commit that referenced this pull request Nov 3, 2026
…an idle close

The pool records what this host pushed to a session under the slot a
connection fills, but only a connection it launched was told which slot that
is. A run picked up from a registered machine is adopted into the slot that
machine's later dispatch acquires -- those pass no parent binding -- so its
records stayed keyed by the client: the idle close #1127 added took them, and
a cleared instance model or Row Effort found nothing pushed and sent nothing
back. That is the sixth round's finding, on the path where the agent outlives
the process.

adopt now names the slot on the client it hands out, as launch does. The test
adopts a connection, records a push against a session, closes it idle, and
reads the record back on the connection the next acquire makes.

Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
LivXue added a commit that referenced this pull request Nov 3, 2026
…an idle close

The pool records what this host pushed to a session under the slot a
connection fills, but only a connection it launched was told which slot that
is. A run picked up from a registered machine is adopted into the slot that
machine's later dispatch acquires -- those pass no parent binding -- so its
records stayed keyed by the client: the idle close #1127 added took them, and
a cleared instance model or Row Effort found nothing pushed and sent nothing
back. That is the sixth round's finding, on the path where the agent outlives
the process.

adopt now names the slot on the client it hands out, as launch does. The test
adopts a connection, records a push against a session, closes it idle, and
reads the record back on the connection the next acquire makes.

Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
0xKT pushed a commit that referenced this pull request Nov 3, 2026
…1134)

## Summary

Refresh an ACP agent's model menu from the agent's own handshake, both
from a manual control at the top-right of the agent card and
automatically when a model picker opens, and preload the Connections,
Agent Hub, and machine rows while the first screen is already up.

The refresh is a new subagents.refresh_models RPC. It runs initialize
plus session/new in a throwaway directory and records the menu without
prompting, so it costs no tokens. A failed read keeps the recorded menu
and returns the reason; automatic reads reuse a menu read in the last
five seconds, manual reads always run, and concurrent asks for the same
row share one handshake. The shared read is shielded, so one asker's
timeout or closed connection ends only its own wait. The answer carries
both menus, derived by the same function the roster uses (acp_menus),
and the picker patches the one row's model and effort choices in place
instead of reloading the roster, because a reload would remount the
sheet and close the open picker.

Reasoning effort becomes its own ACP setting. Raven measures the
thought_level config option when it probes an agent, on the row's own
model because effort menus are per model, stores it on the subagent row
as effort with the agent's effort_choices, validates writes against that
menu, and sends it after the model on every dispatch. A live session's
response names the option id the push goes through for that session,
but its menu belongs to that session's model and never rewrites the
row's: a model switch does not revise the agent-wide snapshot or rebuild
the agent table. The agent card shows an Effort field with an empty "The
agent's default" state. For agents that advertise effort only as session
modes, Raven offers those modes as effort only when every mode id is a
reasoning level (Pi is the measured case), never when the menu contains
permission or editor modes. The value a session opened on and the value
Raven pushed are held per agent and parent binding, so neither a config
hot apply that replaces the backend nor the pool closing an idle agent
strands a live session on a cleared override, and two bindings that
number their sessions alike do not share a record.

All three dispatch lanes (spawn, direct chat, and DAG nodes) resolve the
row's effort through the same manager hook, session_effort_for, with the
same instance they pass to resolve_mode. On an agent whose mode menu is
its effort menu, the row's effort is sent after the Session Tier and
outranks it, and a person's standing mode override on a named instance
outranks the row's effort: the row's value is withheld for that
instance, so the override is the mode the session stays on. On a
thought_level agent the two are separate channels and both are sent.
CONTEXT.md's Session Tier entry states this order, and a Row Effort
entry defines the term.

The Mirasim adapter now advertises effort as a thought_level option
instead of session modes, so its effort no longer shares a channel with
Raven's run-intensity tier. The option lists only the rungs the
session's model takes and is absent for a model that takes none
(glm-5.3-flash), so a Mirasim row's menu is its own model's.
session/set_mode remains as a deprecated alias for clients and snapshots
that read the old menu. Because the adapter advertises no session modes,
the Session Tier does not reach a Mirasim agent once its snapshot is
measured: resolve_tier finds no shared rung (before this change the tier
landed on the adapter's mode menu), and subagents.instance.set_mode on a
Mirasim instance is refused because the agent offers no modes. A Mirasim
turn runs on the row's effort, else on Mirasim's own default.
CONTEXT.md's Session Tier entry says so.

Two additive wire changes: subagents.refresh_models, and
effort/effort_choices on SubagentRow plus effort/clear_effort on
subagents.update. Older clients and stored snapshots keep working
because every new field is optional. The exported subagent config
schema carries the new optional effort field as well.

Agents that advertise neither a thought_level option nor an effort-only
mode menu get no field (generic Claude Code and Codex are the measured
cases). Raven's own agents keep using the Session Tier, and an
unconfigured preset exposes Effort on the card the Connect write
creates, because there is no row to store it on before that.

Changes in the second review round (one commit each):

- An instance's mode override now outranks the row's effort on a
  mode-effort agent (it used to be overwritten by a second set_mode on
  every turn while set_mode reported it applied).
- A model switch no longer relearns menus from its response, so a
  per-model effort menu cannot rebuild the agent table and drop the
  instance model state a later clear needs. Deliberately flipped test:
  test_a_session_response_rewrites_the_effort_menu_and_names_its_option
  pinned that rewrite; it is now

test_a_session_response_names_its_effort_option_per_session_and_leaves_the_row_menu_alone
  and asserts the opposite (relearn returns nothing, the stored menu is
  unchanged, the per-session record follows a renamed option id).
- The session effort record is held per connection rather than per
  agent name and session id.
- The shared menu read is shielded from one asker's timeout.
- The refresh and agent_meta share one menu derivation, pinned by a
  test across four snapshot shapes.
- The Mirasim effort option is per model, and the probe records no
  effort menu for a row whose model takes none.
- CONTEXT.md states that the Session Tier reaches only agents that
  advertise modes, Mirasim included.
- The Mirasim server test harness bounds every request and its teardown
  at 10 seconds, so the test that hung on Windows fails by name instead.
- A refused effort reads as a save on the agent card, not as a failed
  connect.
- The RPC descriptions of effort and effort_choices (and the generated
  clients and the page's type comments) name the mode channel, the
  instance override and the row's own model; they named only the
  thought_level option and said the effort goes out on every turn.

A follow-up refactor after that round's fix landed:

- The instance mode override now has an accessor of its own,
  _instance_mode_override. resolve_mode adds the session tier on a miss
  and returns to its three-argument signature, and session_effort_for
  asks the accessor directly, so neither caller carries an
  override-only flag. resolve_mode's answer for every input is
  unchanged.
- Two test gaps closed: the direct-chat lane now has a Session Tier
  test (the tier is clamped to the agent's rungs before it is sent),
  and _instance_mode_override is tested directly for no instance, no
  override, and a present override.
- test_subagent_mode_wiring's single-reader guard now expects
  _instance_mode_override instead of resolve_mode. That is a structural
  read point rather than a nominal one: resolve_mode sits on top of the
  accessor, so a second direct reader of instance_mode is visible to
  the scan instead of hidden behind a keyword argument.

Changes in the third review round (two blockers):

- A: subagents.update moved an acp row to another model without
  re-measuring its effort menu, so the picker drew the old model's
  rungs: values the new model refuses were accepted, and values it
  takes were refused. A model move now measures the row on the new
  model (verify_agent's handshake) once the model branch has settled,
  and a same-call effort pick is validated against that measurement.
  The measurement is rolled back, stamped, when the call is refused or
  cancelled after taking it, so the store never keeps describing a
  model the row did not adopt.
- B: subagents.refresh_models recomposed the agent table on every read,
  and opening an acp picker triggers that read. Each recompose replaced
  the backend instances, taking the per-instance model baseline and
  pushed value with them, so clearing an instance model sent no
  restore. The model record now lives in the per-connection session
  record beside effort and mode, and a read that measured the
  capabilities the row already held no longer recomposes the table.
  This is another trigger path for the B2 consequence, not a change to
  B2's fix: the first round moved the effort record off the backend,
  this one moves the model record and drops the needless rebuild.

Changes in the fourth review round (two non-blocking notes):

- A same-call model + effort on a never-measured row judged the effort
  against the model the row was leaving: the never-measured branch had
  measured that model, and a failed read of the new model left its
  answer standing for the effort check. The move now clears that claim
  before reading the settled target. (The fifth round replaces the
  second read this round added after a failed one: such an effort is
  now refused as unchecked.) A refused or cancelled write puts the
pre-move record back. The new test makes the move's read fail once and
the
  effort check's read succeed; dropping the model_moved guard makes it
  fail, which the earlier same-call test did not catch.
- An orphaned shared read and the composed table: the table sync lives
  in the shared read, so a read that measured a moved menu still
  reaches the table with every asker gone, and a read that measured
  what the row already held does not recompose it. A test with real
  snapshots drives both halves; the first fails if the sync moves back
  out of the shared read.

Changes in the fifth review round (a self-review of this head; ten
findings, all fixed here):

- A model move whose own handshake failed still judged a same-call
  effort on the old model's menu. record_capabilities answers a failed
  handshake with an unusable snapshot instead of raising, and the record
  it leaves keeps the previous menus, so the move counted as measured. A
  move now counts as measured only when the read is usable and read the
  menu, the test refresh_models applies; otherwise a same-call effort is
  refused as unchecked, with one handshake, and the pre-move record is
  put back.
- A model-only move kept a stored effort the new model does not offer.
  The move's measurement now judges it: a rung the new menu lacks is
  dropped, and kept when the read failed. Before, the agent refused it
  on every turn, and with no menu the card hid the picker, so the value
  could be neither seen nor cleared. The model and effort parameter
  descriptions of subagents.update say so, and the generated clients
  follow.
- A rename onto a taken name with a model move measured under the taken
  name before the duplicate check, overwriting that row's capability
  record, and the rollback then deleted it. The rename is now validated
  (validate_agents) before anything is measured, and the rollback
  restores by the key the measurement wrote, stamp-checked, so a
  built-in row addressed by an older spelling gets its pre-move record
  back instead of losing it.
- An instance moved to another model was sent the row's effort, which
  was only checked against the row's own model. The model switch's
  session response names that model's menu, so a session whose model
  does not offer the effort is not sent it, and the skip is logged once
  per value.
- A model-only move on an enabled row measured the agent and then sent
  it the connect prompt, so an agent that does not start was waited out
  twice. Such a move is now measured after the gate; a move that carries
  an effort is still judged before the gate spends a prompt.
- On an agent whose mode menu is its effort menu, every turn sent the
  Session Tier and then the effort on the same channel. The effort is
  now sent in place of the tier, and the tier only when the agent
  refuses the effort.
- The per-connection session records grew by one per session for the
  life of a pooled connection. A record now goes when nothing can route
  into its session: every turn of a stateless agent, a first turn that
  did not bind, an unsettled cancel, and a pruned resume.
- A mode menu made only of default or none was offered as an effort
  menu. It now needs one level besides those.
- The transport-failure warning for an effort put the session id in the
  effort's slot and the effort in the session's.
- ui-web: the Connections read serialization let every caller waiting on
  one warm read resume together, so two reads ran at once on the shared
  rows and an earlier, slower answer could land last. The wait is now a
  loop.

Kept as it is, on purpose: the effort is still pushed on every turn even
when the session already reports that value. A mode push can move the
effort on the agent side
(test_a_declared_mode_may_move_the_reasoning_effort
pins that for Raven's own adapter), and the mode is pushed every turn,
so skipping on the reported value could leave a session on the mode's
effort.

Changes in the sixth review round (three findings from 0xKT on the
fifth round's head, all fixed here):

- A model move's handshake reverted every other subagents.* write that
  landed while it ran: the list read before the await was written back
  whole. The list is now read once, after the last await (the connect
  gate's prompt or the move's handshake, whichever ran), and only this
  call's own fields, taken against the row as it was before the first
  await, are replayed onto it. A switched-off folder's stub found under
  its name is replaced by the folder's entry, as the write did before
  it re-read the list. The fifth round had moved the model-only
  measurement after the gate's own re-read, which is what exposed
  enabled rows to this; disabled rows had it since the move was first
  measured.
- The session records were keyed by the connection's client, and the
  idle close #1127 added ends exactly the agents that reload their
  sessions: the instance's next turn reloaded the session in a new
  process with an empty record, so a cleared instance model or Row
  Effort was never sent back. Records are now keyed by the pool slot,
  the agent's name and parent binding, and a session opened with
  session/new starts on an empty record, so a relaunched process reusing
  an id inherits nothing.
- A failed menu read still recomposed the whole agent table, because
  the sync compared the stored row with the handshake's raw answer,
  which carries no capabilities when it failed; and it set no cooldown,
  so every picker open relaunched a failing agent. The sync now compares
  the row the store holds after the read, and the cooldown is stamped
  however the read ended. My fifth-round verification of this thread
  covered only reads that succeeded.

Changes in the seventh review round (a fresh review of the sixth round's
head, all fixed here):

- The pool names the slot a connection fills on the connection it
  launches, and the sixth round's session records are keyed by it -- but
  a run picked up from a registered machine is adopted into that same
  slot (a machine's agents acquire with no parent binding) and its
  client was never told which slot it fills. The idle close therefore
  still took a remote agent's records, so a cleared instance model or
  Row Effort there was still never sent back. adopt names the slot too.
- The cooldown stamped on a failed read recorded when it happened but
  not how it ended, so the suppressed ask answered ok: true with no
  detail, and the page dropped the note it had just drawn -- a menu it
  had not read, presented as current. The stamp carries the outcome and
  the detail now, and the suppressed ask answers both. The constant's
  docstring still described the success-only rule this replaced, and
  argued for it; it describes the rule in force.
- A folder's edit that has to handshake re-reads the list before
  writing, and the whole-row replacement that puts the folder's entry
  over its switch stub took the flag from the pre-await copy.
  Switching a discovered folder on removes its stub, so a toggle that
  landed during the handshake left no row to merge onto, and the write
  put the folder's entry back switched off after the toggle had
  answered success. The flag comes from the row on disk now, and a stub
  that is gone comes back on.
- ui-web: a menu read patches its one row so the picker stays mounted,
  and a listing that started before it answered then landed and painted
  the menu as it stood before that read. A listing now replays a menu
  this page read since it started, and drops it once a listing started
  after the patch lands. A failed read's note also outlived the row
  being measured again -- a Test or the sheet's re-check left it over a
  menu it had just read -- so a probe listing and a successful write
  clear it.
- ui-web: the offline library carried no reasoning effort at all, so
  the page's effort field never drew on the one page a reader gets with
  no gateway, and its subagents.update accepted an effort and dropped
  it. One acp row carries a menu and a stored effort now, and the write
  keeps what that menu offers. The card drew the field straight from
  the row, so a pick went back to the old rung for the round trip, and
  a stored rung the re-read menu no longer offers had no option to
  match and drew blank; the value is projected through the write in
  flight and the unoffered rung is drawn as itself.
- gloryfromca's nonblocking finding on this round's own head: taking the
  read sequence in `run` made a write supersede the roster read already
  in flight, and that read returns through the stale guard without
  clearing `loading` -- a flag only a read clears. With rows preloaded
  the page is interactive during its opening probe, so a write pressed
  there left it saying "Checking" and disabled Check again until another
  read started. The write clears it when no newer read has taken the
  sequence since, and a test pins that order.

## Type

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

## Verification

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

Commands and results (Linux, Python 3.12.13, this head, with the
RAVEN_SUBAGENT family unset because this shell runs as a Raven
sub-agent and that role withholds run_subagent_dag):

```text
pytest -n 4, the whole suite (pyproject addopts), at 0f1a64c
17 failed, 36327 passed, 255 skipped, 2 xfailed
  The 17 ids re-run with -n 0 at fc1c1a4 and 0f1a64c:
  the same 9 fail on both (provider proxy redaction x5, install script
  node dir, legacy Excel workbook, one oncall refused submit, node
  runtime) and the other 8 pass on both. None touches a changed file.

pytest -n 0 -k "effort or menu or relearn or mode or tier or override
or refresh or mirasim or ..." over the eight touched test files, with
each of the nine new commits archived on its own
911, 918, 920, 922, 924, 927, 928, 930, 930 passed

At e2d6df9 (this refactor; its parent is 0f1a64c), -n 0:
  the four focused files (test_subagent_manager.py,
  test_subagent_mode_wiring.py, test_subagent_mode_resolution.py and
  test_rpc_instances.py)
  413 passed
  test_subagent_acp.py -k "effort or mode"
  44 passed, 326 deselected
  test_subagent_direct_chat.py, test_subagent_third_party.py,
  test_agent_loop_iteration_hooks.py, test_subagent_acp.py and
  test_subagent_dag_runner.py
  1165 passed

At 545c6b9 (this round; its parent is e2d6df9), -n 0:
  tests/test_rpc_subagents.py, test_rpc_subagents_model.py and
  test_subagent_acp.py
  602 passed
  tests/test_acp_backend_cwd.py, test_acp_unprompted.py,
  test_acp_mirasim_stream.py, test_acp_client_resume.py,
  test_acp_methods.py, test_acp_mirasim_server.py,
  test_remote_resume.py, test_resume_remote.py,
  test_subagent_mirasim.py,
  test_subagent_acp_mcp_lifecycle.py, test_subagent_github_copilot.py,
  test_subagent_acp_remote.py and test_harness_curator_hosting.py
  528 passed
  tests/test_subagent_manager.py, test_subagent_third_party.py,
  test_subagent_direct_chat.py, test_rpc_instances.py and
  test_core_runtime_swap.py
  765 passed
  tests/test_subagent_dag_runner.py
  400 passed
  the three B4 pins by name:
  test_refresh_models_reads_the_handshake_and_returns_that_menu,
  test_a_mode_derived_effort_menu_travels_with_the_refreshed_models and
  test_a_session_response_names_its_effort_option_per_session_and_leaves_the_row_menu_alone
  all passed

At 5a96404 (this round; its parent is 545c6b9), -n 0:
  tests/test_rpc_subagents.py, test_rpc_subagents_model.py and
  test_subagent_acp.py
  604 passed
  tests/test_subagent_manager.py, test_subagent_third_party.py,
  test_subagent_direct_chat.py, test_rpc_instances.py and
  test_core_runtime_swap.py
  765 passed
  the twelve ACP and harness files listed above
  528 passed
  tests/test_subagent_dag_runner.py
  400 passed

npx vitest run --no-file-parallelism (ui-web, node 26 with
--localstorage-file)
260 files, 4452 tests passed

npx vitest run src/features/extAgents/health.test.ts
fc1c1a4 health.ts: 1 failed (expected 'connect' to be 'save'); here 8 passed

ruff check, ruff format --check (Makefile targets): clean, 2460 files
lint-imports: 10 kept, 0 broken
ty check: all checks passed
npm run gen:check / lint / type-check (ui-web): clean; the 4 eslint
warnings are in files this PR does not touch
npm run lint:rpc / type-check (ui-tui): in sync, clean
scripts/check_commit_messages.py origin/main..HEAD: ok
scripts/check_large_files.py origin/main...HEAD: ok
scripts/check_source_language.py origin/main...HEAD: ok
scripts/check_pr_body.py on this description: ok
```

New tests, red before the fix and green after: the 21 written first
gave 16 failed and 5 passed at fc1c1a4 (the 5 are the thought_level
control and the four menu-derivation pins, which agree at that head by
design), and the two harness-bound tests hung there; all pass at this
head. Mutants, each restored by sha256 afterwards, all killed: no
channel check in session_effort_for, no override rank at all, the
effort_via_modes flag never set or dropped by the registry, the effort
record keyed by agent instead of connection, the relearn effort branch
restored, the model-switch relearn restored, the option id first-wins
again, no shield, the refresh deriving effort inline, the Mirasim
catalog ignoring the model, the probe keeping the default model's menu
on a no-effort model, an unbounded harness request, and the
override-only answer falling back to the tier. Two survivors on the
first pass were closed by stronger tests before this push: the
model-switch relearn (the switch test's response now carries a moved
model menu too) and the tier fallback (a test with a Session Tier and no
override). The full suite also caught test_subagent_mode_wiring's
single-reader guard on instance_mode; the override now has one
accessor, _instance_mode_override, which session_effort_for asks
instead of reading the override itself, and the guard's expected
reader set names that accessor.

The whole suite was not re-run at this refactor commit; every file it
touches is exercised by the focused and related runs above, and no
other file changed.

This refactor's mutants, each restored by sha256 afterwards, all
killed: a second direct instance_mode read in session_effort_for (the
single-reader guard), the chat lane dropping its resolved mode, the
accessor never returning a stored override (only the override-present
case fails), and session_effort_for skipping the override rank.

This round's RED, each restored by sha256 afterwards: disabling the
model-move re-measure made all three new update tests fail, and putting
the model record back on the instance as per-backend dicts made both
clear tests fail with no restore frame. The no-change sync test was
written first and failed against the unconditional recompose.

The non-blocking round's RED: the new same-call test failed before the
fix (the old menu accepted the rung and the write landed), and failed
again with (model_moved and not measured_for_effort) dropped, while
the earlier same-call test still passed under that mutant. The orphan
test failed with the table sync moved back out of the shared read and
passed with it in place.

The Windows hang, simulated on Linux by a stand-in host whose hold
ignores a stop: the old harness ran
test_a_turn_stopped_from_the_app_ends_cancelled until a 60 second
timeout killed it with no result; the bounded one failed it after 12.9
seconds with "session/prompt on <session> got no answer from the
adapter within 10s" and a clean teardown. Not re-run on Windows at this
head.

Fifth round, on this branch rebased onto main 945ddd7 (Linux,
Python 3.12.13):

```text
pytest -n 16, the whole suite, with coverage
102 failed, 37582 passed, 263 skipped, 2 xfailed
  The same 102 ids fail on the rebased head without this round, in the
  same environment. 94 of them are node and remote tests that fail only
  because that run imported the tree through PYTHONPATH while the venv's
  editable install pointed at another checkout ("runs different code
  from this Raven"); run from the checkout the venv belongs to, those
  files pass and only the 8 known failures remain (provider proxy
  redaction x5, install script node dir, legacy Excel workbook, node
  runtime).

scripts/coverage_gate.py diff --base-ref origin/main --threshold 90
98.24% (501/510); none of the 9 uncovered lines is from this round

tests/test_subagent_acp.py, test_rpc_subagents.py,
test_rpc_subagents_model.py, test_subagent_manager.py,
test_subagent_mirasim.py, test_acp_mirasim_server.py,
test_acp_mirasim_catalog.py, test_subagent_dag_runner.py,
test_subagent_mode_resolution.py and test_subagent_mode_wiring.py
1522 passed

npx vitest run (ui-web): 276 files, 4759 tests passed
npm test (ui-tui, after npm ci): 146 files, 2288 passed, 13 skipped
ruff check, ruff format --check: clean
lint-imports: 10 kept, 0 broken
ty check: all checks passed
npm run gen:check / lint / type-check (ui-web): clean
npm run lint / lint:rpc / lint:i18n / lint:shared / type-check / build
(ui-tui): clean
scripts/check_source_language.py <merge-base>...HEAD: ok
```

This round's new tests were each red before their fix and green after.
Sixteen mutants, each undoing one fix and restored by sha256 afterwards,
were all killed: a failed read counted as measured, no drop of an
unoffered stored effort, a model-only move measured before the gate, no
early rename check, the rollback keyed by the request's name, no
session-menu check before an effort push, the tier always sent ahead
of a mode-channel effort, no tier fallback after a refused effort, each
of the five places a session record is dropped, the neutral-mode check
removed, the warning's arguments swapped back, and the Connections wait
back to a single await.

Sixth round, on the same base (main 945ddd7; Linux, Python 3.12.13),
run from the checkout the venv belongs to:

```text
pytest -n 16, the whole suite, with coverage
8 failed, 37686 passed, 263 skipped, 2 xfailed
  The same 8 fail on the fifth round's head in the same environment
  (provider proxy redaction x5, install script node dir, legacy Excel
  workbook, node runtime).

scripts/coverage_gate.py diff --base-ref origin/main --threshold 90
98.29% (516/525); none of the 9 uncovered lines is from this round

the ten files above, test_acp_pool_idle_close.py,
test_acp_client_resume.py and test_subagent_acp_remote.py
1618 passed

ruff check, ruff format --check: clean
lint-imports: 10 kept, 0 broken
ty check: all checks passed
git diff --check: clean; no added line carries a non-ASCII character
```

This round's new tests were each red before their fix and green after.
Eight mutants, each undoing one fix and restored by sha256 afterwards,
were all killed: no merge after the awaits, the whole row replayed
instead of the call's fields, a merge onto the switch stub, records
keyed by the client again, the pool setting no slot, a fresh session
inheriting the record, the sync comparing the handshake's answer, and no
cooldown after a failed read.

Seventh round, rebased onto main d056204 (Linux, Python 3.12.13), run
from the checkout the venv belongs to:

```text
The whole suite on this head is CI's unit job: four shards, all passed,
with coverage gates (90% diff coverage) passing behind them. A local run
of it wedged three times in this environment inside test_node_server.py's
real-process tests; those files pass on their own, and the same tests
pass in CI, so the shards are the full-suite answer for this head.

the eight affected files (test_subagent_acp.py, test_rpc_subagents.py,
test_rpc_subagents_model.py, test_remote_resume.py,
test_acp_client_resume.py, test_subagent_manager.py,
test_subagent_mirasim.py, test_acp_mirasim_catalog.py)
1010 passed

npx vitest run (ui-web): 277 files, 4772 tests passed
npx tsc --noEmit (ui-web): clean
node scripts/gen-rpc-client.mjs --check, ui-tui gen-rpc-types --check:
  both in sync (249 methods / sha256 baa1574f1f68)
ruff check, ruff format --check: clean
lint-imports: 10 kept, 0 broken
```

The rebase conflicted in one place: main's #1146 clears a `picked` model
when a connect lands, and this round clears a failed menu read's note in
the same `if` chain. Both lines are kept, and main's test for the pick
passes.

This round's new tests were each red before their fix and green after.
Ten mutants, each undoing one fix and restored by sha256 afterwards,
were all killed: adopt without the slot, the cooldown stamped only on a
successful read, the suppressed answer back to ok, the folder written
back with the pre-await flag, a listing painting over the menu just
read, a probe listing leaving the failed read's note, a successful
write leaving it, the offline library's effort back to null, and the
unoffered rung drawing blank, and the superseding write leaving
`loading` set.

The review's three probes run in both directions, on the same shapes:
an adopted connection's push is gone after close_idle + acquire at the
previous head and still there at this one; a failed read's suppressed
answer is ok: false with the reason only after 4e532e4; and a toggle
that lands during a folder's effort handshake survives only after it.

## Risk

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

No secrets are added; effort values are agent-supplied menu ids and
reach only the agent's own config. The changes are additive: a new RPC
method, optional row fields, and an optional config field. Stored
snapshots measured before this change keep their mode-derived effort,
and session/set_mode is still accepted by the Mirasim adapter. A Mirasim
snapshot measured before this change still carries the old mode menu, so
the Session Tier keeps reaching that agent until the snapshot is
measured again (Test, a menu refresh, or a launch-config change).
The manager ranks an instance override against the row's effort by
the row's measured menu, while the backend picks the channel per
session: if a live session of a mode-effort agent ever reported a
thought_level option of its own, the row's value would still be withheld
for an overridden instance although that session could take both. No
agent measured so far does this.
Reverting this change leaves no migration or on-disk format behind.

This round adds one handshake (a launch and session/new, no prompt) to
every acp subagents.update that moves the row's model, in addition to
the connect ping the same call already sends. The measurement is taken
before the call can refuse, so a concurrent reader can see the new
model's menu for the duration of an update that is then refused or
cancelled; the rollback puts the previous row back, stamped so a newer
Test or refresh stands. A cleared instance model relies on a record
held per agent and parent binding, which survives the pool closing the
idle agent and reloading the session in a new process; a Raven restart
starts it empty, as it did before. A move whose own read fails costs no
second
handshake: an effort sent with it is refused as unchecked, and a
model-only move keeps its stored effort. A model-only move on an
enabled row is measured after the connect gate, so a refused move
spends one launch.

User-visible behaviour changed in the fifth round: a model move drops
a stored effort the new model does not offer; an instance on another
model is not sent the row's effort its model lacks; on an agent whose
modes are its effort menu, the Session Tier is no longer sent ahead of
an effort. Each is one revert away.

The manual and automatic reads launch the agent process, bounded by the
RPC's 60 second timeout; a failure keeps the previous menu and reports
the reason. The effort field is omitted for rows whose agent advertises
no usable menu.

Sixth round: a failed automatic menu read now holds the five-second
cooldown like a successful one, so a picker reopened within it shows
the recorded menu without launching the agent; the refresh control
still reads. Session records outlive a connection: they are kept per
agent and parent binding until no instance can resume the session.

Seventh round: an adopted connection now names the slot a launch does,
so a remote agent's session records also survive the idle close; before
this only launched connections did. The cooldown reports how the read it
stands in for ended, so a picker reopened inside it shows the recorded
menu with the reason. A folder's switch is read from disk on the write
that re-reads the list, so an edit cannot undo a toggle that landed
during its handshake. A write now clears the checking state its own
sequence leaves behind, so a write pressed while a page's opening probe
is out no longer strands that flag. The offline page carries one row
with a reasoning effort; nothing else about it changes.

## Related Issues

N/A

---------

Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
@0xKT 0xKT mentioned this pull request Nov 3, 2026
4 of 12 tasks
LivXue pushed a commit that referenced this pull request Nov 3, 2026
## Summary

Bump the package version from 0.2.6 to 0.2.7 for the next release. Only
pyproject.toml and uv.lock change.

Since v0.2.6, main merged 48 PRs: 17 features, 27 fixes, two docs, one
perf
and one chore. The larger features are a task board per conversation
with a
Task Board page across them (#1157), per-conversation reasoning effort
(#1113), handing a dag node to another sub-agent mid-run (#1118), raven
agents sharing memory with their host and each other (#1127), the agent
trajectory redrawn as a waterfall with external agents' steps and replay
(#1147, #1156), deepseek as a web search and fetch provider (#1125),
Skill Hub retrieval with a settings switch (#1141), compressed trace
payloads and cold logs (#1149), and a per-section `tls_verify` opt-out
for
self-signed model gateways (#1108). None carries a BREAKING CHANGE
footer.
The maintainer chose a patch bump, the way v0.2.1 through v0.2.6 were
for
their features.

A v0.2.7-rc1 tag goes first to dry-run the release pipeline, as v0.2.6
did.
The stable tag and release notes follow once this lands.

uv.lock is regenerated with uv 0.12.12, which leaves only the raven
version
line changed (uv 0.9.x rewrites unrelated dependency markers and uv
0.13.x
raises the lock revision).

## Type

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

## Verification

- `uvx --from uv==0.12.12 uv lock` on a clean worktree cut from
origin/main:
  Updated raven v0.2.6 -> v0.2.7
- `git diff --numstat origin/main..HEAD`: one line each in
pyproject.toml and
  uv.lock
- `uvx --from uv@latest uv lock --check` (uv 0.13.7): exit 0, so the
  `uv export --locked` step in release.yml accepts the lock
- `PYTHONPATH=. python3 scripts/check_commit_messages.py
origin/main..HEAD`:
  exit 0
- `npx commitlint --from origin/main --to HEAD --config
commitlint.config.cjs`:
  exit 0
- No test run: the change is version metadata only.

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

## Risk

No code change. Rollback is reverting this commit before any v0.2.7 tag
is
pushed.

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

## Related Issues

N/A

Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
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