fix(agent): a pick-up binds a cli instance the way a dispatch does by Handsome-wzw · Pull Request #979 · EverMind-AI/Raven

Handsome-wzw · 2026-10-16T03:43:11Z

Summary

The two code notes LivXue left on #957 about _bind_picked_up (the pick-up of a cli create left running on a registered machine), and the two comments the same review found stale.

A stateless run binds nothing at a pick-up. The gate read not run.handle, which never fires: every run carries a handle (instance or task_id), and a stateless entry may declare idSource: derived too, which the dispatch path reads nothing from and binds nothing with. The pick-up now reads the record's stateful, the flag the dispatch wrote, so a stateless derived run is picked up with no binding and no "not resumable" warning, as _attempt leaves it.
A binding a dispatch made while the pick-up waited is kept. The pick-up committed without the handle's lock and without a lookup; a dispatch to the same named instance during the window looked up, found nothing, created and bound, and the pick-up then overwrote that with the picked-up create's session, moving the instance back to a thread the newer turn never saw. _bind_picked_up now takes hold_handle around a lookup and the commit, as run -> _run_stateful does, and keeps any binding it finds (with no warning: the instance is resumable through it).
The binding is one atomic step, for both id sources, and the pick-up waits for no turn (the review's two notes on this PR's first commit). InstanceRegistry.bind_if_absent binds a handle only where nothing is bound, under the registry's own lock, and returns what the handle is bound to after; the cli pick-up (derived) and resume_remote._conclude (the minted id of the default provisioned entry, which had the same race) both use it. The pick-up takes no hold_handle: the dispatch holds that across its whole turn with no timeout, and the pick-ups run one after another, so waiting there would have held back every run picked up after it. The dispatch commits last whichever way the two land, so the handle ends on its session in every interleaving.
Two comments in manager.py (announced = True and the finally) said the kept record was a refused reply's only; since fix(*): machine runs outlive a killed daemon, a refused failure announce, a dead-link cancel #957 a refused failure announce keeps it too, and the comments say so now. CHANGELOG's fix(*): machine runs outlive a killed daemon, a refused failure announce, a dead-link cancel #957 entry gained the same clause.

Thread links: #957 (comment) (stateless gate), #957 (comment) (lock and lookup), #957 (comment) (comments). The third note on that PR, the stderr replay, is #977.

Follow-up commit, after the review of this PR:

A left turn that resumed the session its handle is bound to closes its row. bind_if_absent found the handle bound to that very id and wrote nothing, so the running the manager put on the row when the turn began stayed and the instance read as interrupted after a reply that was announced (the merge base's commit rebuilt the row and dropped it). It now rebuilds a row bound to this very id too, as a binding alone; a row bound to another id is still that turn's.
The pick-up finishes its own record. _spawn_record took the latest running record of the agent and handle, so a newer turn to the same named instance, which opens its record as its dispatch starts, could have the left run's reply written into it and its node closed. The record the manager named when it left the run (node_id in the run's origin) is taken first; the latest running is the guess only for a run a host killed outright left unannotated.

Thread links: #979 (comment) (the row), #979 (comment) (the record).

Type

Verification

Each fix has a test that failed first on the tree before it: the pick-up binds the id its output names; a stateless derived run binds nothing and warns of nothing; a newer binding is kept; the interleaving itself against the real InstanceRegistry (the pick-up completes while a dispatch still holds the handle, and the handle ends on the dispatch's commit); bind_if_absent binds once and reads a status row or another kind as absent; _conclude keeps a newer binding for a minted id. The first three fail on main; the last three fail on this PR's first commit.

.venv/bin/python -m pytest -n 6 -q <the 21 test files that touch the cli backend, hold_handle, the instance registry, the remote run records or the pick-up> - 1842 passed; changed-line coverage 100% (23/23, scripts/coverage_gate.py diff --base-ref origin/main). Full suite: 32047 passed, 174 skipped, 2 xfailed in 3 min 50 s; the 3 failures are the ones main has on this machine (a host without LibreOffice, two simulation records).
ruff check / ruff format --check, ty check on the two changed modules, lint-imports (10 contracts kept), scripts/check_commit_file.py, scripts/check_source_language.py, scripts/check_large_files.py, pre-commit: clean.
Real machine (ravenx2, isolated registry copies and Raven homes, this branch's node and main's node under their own node_dir; the same script on both trees):

Probe	`main`	this branch
F3: a named derived create left running; the next start picks it up; a dispatch to the same instance runs during the pick-up; then one more turn	the pick-up overwrote the dispatch's binding with its own session, and the next turn resumed the old session (`resumed first-<pid1>`)	the dispatch's binding kept, and the next turn resumed it (`resumed first-<pid2>`); the pick-up's own session left unbound
F2: a stateless derived create left running, picked up by the next start	picked up ok, and bound the handle to `s-<pid>`	picked up ok, nothing bound, no warning
F1: a provisioned (default) named create left running; a dispatch to the same instance during the pick-up; then one more turn	`_conclude` bound the minted id over the dispatch's, and the next turn resumed the minted session	the dispatch's session kept, and the next turn resumed it; the minted id left unbound
F2T: a dispatch holding the handle for ~25 s while the left run's command had already ended (this PR's first commit vs this head)	the pick-up's announce came at +25.4 s, only when the newer turn released the handle at +25.4 s	the pick-up announced at +5.0 s (as soon as its command's output was read) while the newer turn ended at +25.4 s; the handle ended on the newer turn's session

Docs checked against the code before pushing: the _bind_picked_up, bind_if_absent and _track_on_machine docstrings, the comment in _conclude, the two manager.py comments, CONTEXT.md's Raven node entry (unchanged: it describes the pick-up without the binding detail), and the CHANGELOG entry.

Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Follow-up commit 1f4fa74: the three tests above fail on fc57b79 (status running kept; the newer
record finished) and pass now; tests/test_resume_remote.py, tests/test_subagent_cli_remote.py,
tests/test_subagent_acp_remote.py, tests/test_subagent_manager.py, tests/test_remote_runs.py,
tests/test_remote_resume.py: 372 passed. Full suite on the merged tree: 32061 passed, 173 skipped,
3 failed (the same 3 that fail on main on this computer: LibreOffice and two simulation cases). Diff
coverage 93.18% (41/44). ruff, ty, lint-imports and the three range checks on origin/main..HEAD clean.
main merged in first (test(tests): put the EverOS role binding back after every test #964, fix(*): offer the host's folder dialog only where the gateway has one #970, fix(ui-web): let the desk palette reach down to the composer card #980), no conflicts.

Risk

Security impact considered - none: the pick-up binds strictly less than before (never for a stateless run, never over a binding already made), through one step under the registry's own lock.
Backward compatibility considered - records written before this change carry stateful already (feat(*): pick up a turn left on a registered machine when this computer stops #934); no format change.
Rollback path is clear for risky changes - a plain revert.

Related Issues

Follow-up to #957 (the three nonblocking notes of its review).

Two notes from the review of EverMind-AI#957 on `_bind_picked_up`: - The gate for "a stateless run" read `not run.handle`, which never fires: every run carries a handle (`instance` or `task_id`), and a stateless entry may declare `idSource: derived` too, which the dispatch path reads nothing from and binds nothing with. The record's `stateful` is what tells the two apart, and the pick-up reads it now. - The pick-up committed its binding without the handle's lock and without a lookup, so a dispatch to the same named instance that ran while the pick-up waited on the machine -- looked up, found nothing, created, bound -- had its binding overwritten with the picked-up create's session, moving the instance back to a thread the newer turn never saw. The pick-up now takes `hold_handle` around a lookup and the commit, as the dispatch path does, and keeps a binding it finds. Two comments in the manager that explained the kept record for a refused reply only now say the failed branch keeps it too. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>

gloryfromca

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

I reviewed the full diff and the surrounding CLI dispatch, remote-run persistence, pick-up, instance-registry, and handle-lock paths. The new stateful gate matches the persisted run capability, and the lookup/commit now follows the dispatch ordering without overwriting a binding made by a newer turn. I also checked the relevant AGENTS.md and Runtime terminology rules, history and callers, backward compatibility of the existing RemoteRun record, and that the prior test was expanded rather than weakened. The manager-only edits accurately update existing comments.

Verification: uv run --extra dev pytest tests/test_subagent_cli_remote.py -q (56 passed). A first run without the repository's dev extra did not reach test bodies because the shared fixture could not import raven_everos; rerunning with the declared dev environment passed. git diff --check github/main...HEAD also passed.

LivXue · 2026-10-16T04:28:27Z

Windows compatibility review of head 31a71315f11c6411aa66d8a50ada160a4eb213a0 against base 400b30dfa369a094e80949c6860a7d215c835e07.

No actionable Windows compatibility issue found in the inspected four-file change. Read the whole diff and traced pick-up/dispatch callers, persisted RemoteRun.stateful, hold_handle, registry reloads and atomic writes. The new guard uses the persisted capability; lookup and commit follow the existing asyncio handle lock. Registry writes retain the existing Windows-capable file lock and explicit UTF-8 encoding. Manager and changelog edits agree with the runtime.

Native Windows verification, Python 3.13.12, isolated full-SHA source and environment:

Repository selection below: 5 passed, 4 skipped. The four pick-up tests inherit a /bin/sh-dependent box fixture, so their native Windows skip is a coverage limitation.
External native probe: 5 passed. It invoked those same four regression bodies with a temporary machine-registration file and their in-memory channel endpoints, avoiding the POSIX fixture. The fifth case exercised real registry persistence/reload under a Unicode/space/%20 Windows path, including Unicode identifiers and CRLF output.
Ran the stateless and concurrent-dispatch probes separately against the exact base: both failed at the intended assertions, while both pass at this head. This adds native Windows evidence for the stateless gate and binding-order fix.

Verification commands

$scratch was C:/Users/livxu/AppData/Local/Temp/raven-windows-review/b75bcfc86cc84e0bba24c830e503204e. Each pytest command appended -n 0 -x --tb=short --basetemp "$scratch/checks/SUITE/pytest" --junitxml "$scratch/checks/SUITE/junit.xml", with the suite named below.

Setup: uv sync --frozen --python 3.13 --all-packages --extra dev.

Repository selection (979-repo-pickup):

uv run pytest `
  tests/test_subagent_cli_remote.py::test_a_picked_up_create_binds_the_instance_to_the_session_id_its_output_names `
  tests/test_subagent_cli_remote.py::test_a_picked_up_stateless_run_binds_nothing_and_warns_of_nothing_as_the_dispatch_path_does `
  tests/test_subagent_cli_remote.py::test_a_binding_a_dispatch_made_while_the_pick_up_waited_is_kept `
  tests/test_subagent_cli_remote.py::test_the_pick_up_waits_for_a_dispatch_that_holds_the_handle_and_then_keeps_its_binding `
  tests/test_subagent_instance_registry_reload.py

Native probe (979-native-pickup): uv run pytest -c pyproject.toml "$scratch/test_windows_pickup.py".

Base controls (979-base-stateless, 979-base-serialize), each run separately:

uv run pytest -c pyproject.toml "$scratch/test_windows_pickup.py" -k stateless
uv run pytest -c pyproject.toml "$scratch/test_windows_pickup.py" -k serialize_dispatch

Coverage limits: no live SSH/node or vendor CLI execution, and no full-suite compatibility claim. All four changed source blobs were verified against GitHub's head metadata; the user's active checkout was unchanged.

LivXue · 2026-10-16T05:00:01Z

No blockers in the reviewed scope.

Scope: I reviewed only the raven/agent/** portion of this PR -- 2 of 4 changed files (raven/agent/subagent/backends/cli_agent.py, raven/agent/subagent/manager.py). The other 2 files (the tests and CHANGELOG.md) were read as context but are not covered by this verdict.

What I examined: CliAgentBackend._bind_picked_up, the one method changed, against its caller (pick_up, cli_agent.py:1367), the dispatch path it now mirrors (_launch and _attempt's deferred commit at :1469-1472), hold_handle, the run records' stateful field, and resume_remote_runs and _conclude in raven/agent/subagent/resume_remote.py; and the two manager.py comments, against the code they describe. This answers my three notes on #957. The stateless gate now reads the record's stateful (:1400), so a stateless entry declaring idSource: derived is neither bound nor warned at a pick-up, as on the dispatch path. The binding is made under the handle's lock after a lookup (:1408-1416), so a binding a newer dispatch made while the pick-up waited is kept. Both hold for derived: the PR's tests for them pass on this head's tree, and the probe in F1's note shows the second against the merge base. The manager.py comments (:1808-1811, :1979-1984) now match the code: an announce the draining scheduler refuses keeps the record, after a reply or a failure (:1904, :1953, :1968, :1985-1986).

Confirmed findings:

F1, Nonblocking, inline at cli_agent.py:1400 -- for the default idSource: provisioned the same race stays open: resume_remote._conclude commits the minted id with no lock or lookup (resume_remote.py:163-164) and moves a newer binding back. It predates this PR.
F2, Nonblocking, inline at cli_agent.py:1408 -- the pick-up now waits for the handle's lock, which a dispatch holds across its whole run with no timeout by default, and resume_remote_runs awaits each pick-up in turn, so every run picked up after it waits for that turn too.

Verification:

F1 and F2: a pytest probe on git archive trees of this head and of the merge base 400b30dfa369, each run printing its modules' __file__, reusing this PR's test helpers and a real InstanceRegistry; the results are in the two notes.
The PR's four pick-up tests pass on this head's tree. The tests in the same files that need the stand-in machine's node cannot run from an archive tree here (that node reports an older protocol than the archive's host) and fail the same way on both trees. CI is green at this head.

Checked and not reported: the removed not run.handle guarded nothing, since a run's handle is always instance or task_id (cli_agent.py:837). RemoteRun.stateful has been a required field since the run records were introduced, and a record without it is dropped on load, so no old record reads as stateless. hold_handle is re-entrant within a task and the pick-up holds no other handle, so the new wait cannot deadlock. A pick-up now looks up any handle, where the dispatch path looks up only a named one; the probe kept an old binding for a handle reused across DAG graphs, but only from a hand-made record, since spawn, DAG and direct chat mint a unique handle for every stateful run they do not name.

gloryfromca

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

The two new inline comments are valid follow-ups: provisioned-id recovery retains a pre-existing overwrite race in _conclude, and the new handle-lock wait can head-of-line-block the serial recovery loop behind a concurrent same-handle turn. Neither needs to hold this improvement, so my stance changes from clean to nonblocking.

Verification: uv run --extra dev pytest tests/test_subagent_cli_remote.py -q (56 passed). No code was changed in this response.

…or the handle Two notes from the review of this PR's first commit: - The default `idSource: provisioned` had the same race the first commit closed for `derived`: the minted id is bound by `resume_remote._conclude`, which committed with no lock and no lookup, so a dispatch to the same named instance that bound a new session while the pick-up waited was moved back to the old one. - The first commit took `hold_handle` around its lookup and commit. A dispatch holds that lock across its whole turn, with no timeout by default, and `resume_remote_runs` awaits each pick-up in turn, so a pick-up of a command that had already ended waited for the newer turn, and every run picked up after it waited too. Both go through one atomic step now: `InstanceRegistry.bind_if_absent`, under the registry's own lock, binds the handle only where nothing is bound and returns what it is bound to after. The dispatch commits last whichever way the two land, so the handle ends on its session in every interleaving, and no pick-up waits for a turn. The cli pick-up and `_conclude` both use it; a status row with no id, or a row of another kind, reads as absent, as `lookup` reads it. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>

gloryfromca

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

I reviewed the delta from 31a71315f11c and the complete PR diff, including the CLI and ACP recovery callers, registry transaction and reload behavior, dispatch ordering, history, backward compatibility, and the applicable AGENTS.md and Runtime architecture terms. I also checked that the tests were strengthened for both fixed interleavings rather than weakened. The two previous nonblocking follow-ups are resolved by the atomic bind_if_absent path without introducing the serial recovery delay.

Verification: uv run --extra dev pytest tests/test_subagent_cli_remote.py tests/test_resume_remote.py tests/test_subagent_acp_remote.py -q (131 passed); uv run --extra dev pytest tests/test_subagent_instance_registry_reload.py -q (5 passed); git diff --check github/main...HEAD passed.

LivXue · 2026-10-16T07:58:58Z

No blockers in the reviewed scope.

Scope: I reviewed only the raven/agent/** portion of this PR -- 4 of 8 changed files (raven/agent/subagent/backends/cli_agent.py, raven/agent/subagent/instances.py, raven/agent/subagent/manager.py, raven/agent/subagent/resume_remote.py). The other 4 files (three test files and CHANGELOG.md) were read as context but are not covered by this verdict.

What I examined: fc57b7912, the commit since my last round at 31a71315f11c. It adds InstanceRegistry.bind_if_absent, which binds a handle only where nothing is bound, in one step under the registry's own lock (instances.py:156-179), and commit now writes through the extracted _bind (:181-202). The cli pick-up binds through it instead of taking hold_handle (backends/cli_agent.py:1414-1421), and so does _conclude for the record's session id (resume_remote.py:163-168). Every picked-up run ends in _conclude (resume_remote.py:103), so the change reaches acp entries as well as cli ones. Line numbers below are this head's.

Findings from my last round:

F1 (the race left open for provisioned) is fixed: with a binding committed while the pick-up waited, a provisioned entry keeps it after _conclude on this head, where 31a71315f11c and the merge base move it back to the minted id; with nothing bound, the minted id is bound on all three trees.
F2 (the serial pick-up waiting for the handle) is fixed: with another task holding the handle for 1.5 s, the two left runs are announced at +0.06 s and +0.07 s on this head, against +1.51 s on 31a71315f11c, with the same final bindings.
@Handsome-wzw resolved both threads, and they stay resolved.

New, Nonblocking, inline:

F3 at resume_remote.py:168 -- a left turn that resumed the session its handle is bound to now leaves the instance's row running, shown as interrupted, because bind_if_absent writes nothing where the merge base's commit rebuilt the row. A regression against the merge base, for cli and acp entries.
F4 at resume_remote.py:161 -- _spawn_record can pick the record of a newer turn to the same instance that is still running, and the left run's reply is written into it. It predates this PR; with the wait gone, the interleaving this PR is built for reaches it.

Verification:

A pytest probe on git archive trees of this head, of 31a71315f11c and of the merge base 400b30dfa369, each run printing its modules' __file__, reusing this PR's test helpers and a real InstanceRegistry: F1 and F2 as above, F3 and F4 as described in their notes.
With this head's tests over 31a71315f11c's code, test_the_pick_up_does_not_wait_for_a_dispatch_holding_the_handle_and_its_binding_still_wins fails with TimeoutError, as the author says. On this head's tree, tests/test_resume_remote.py, tests/test_subagent_instance_registry_reload.py and the five pick-up and registry tests in tests/test_subagent_cli_remote.py pass (30). CI is green at this head.

Checked and not reported: the pick-up and the dispatch end on the dispatch's session in whichever order they land, since the dispatch's deferred commit does not look up (backends/cli_agent.py:1476-1477); where the dispatch commits nothing (a failed or cancelled create, or no id in a derived output), the pick-up's binding stays, as on 31a71315f11c and the merge base. bind_if_absent does not await inside write_transaction, the cross-process write lock, and treats a status row or a row of another kind as unbound, as lookup does. The refactored commit writes what it wrote before. In test_a_minted_id_is_bound_only_where_nothing_is_bound_so_a_newer_turn_s_binding_is_kept, the half that guards the newer binding (tests/test_resume_remote.py:532) also passes on 31a71315f11c, because the stand-in's commit raises AssertionError and _conclude's except Exception (resume_remote.py:171) swallows it; the test fails there only at its second half (:544), and test_bind_if_absent_binds_once_and_reads_a_status_row_or_another_kind_as_absent covers the real registry.

gloryfromca

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

Two newly reported recovery follow-ups are valid: preserving an identical session binding leaves the instance row at running, and selecting the latest running spawn record by agent and handle can attach a recovered result to a newer concurrent turn. Neither blocks this improvement, but they change my stance on this revision from clean to nonblocking.

Verification: uv run --extra dev pytest tests/test_resume_remote.py tests/test_subagent_cli_remote.py tests/test_subagent_acp_remote.py -q (131 passed). No code was changed in this response.

…ollow_ups

Two notes from the review of this PR, both reproduced there. A left turn that resumed the session its handle is bound to reached bind_if_absent with that same id bound, and it wrote nothing: the running the manager put on the row when the turn began stayed, and the instance read as interrupted after a reply that was announced. The merge base's commit rebuilt the row and dropped it. bind_if_absent now rebuilds a row bound to this very id too, as a binding alone; a row bound to another id is still left to that turn. _spawn_record took the latest running record of the agent and handle, so a newer turn to the same named instance, which opens its record as its dispatch starts, could have the left run's reply written into it and its node closed. The record the manager named when it left the run (node_id in the run's origin) is taken first; the latest running is the guess only for a run a host killed outright left unannotated. Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>

gloryfromca

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

I reviewed the delta from fc57b7912d59 and the complete PR diff after the main merge, including recovery callers, registry status ownership, spawn-record history, backward compatibility, tests, and the applicable repository and Runtime rules. The two reported failures are fixed for their reproduced paths. Two narrow follow-ups remain: recovery can clear a newer turn's running status when both use the same session id, and an already-completed named record can fall back to a newer running record on a later recovery cycle. Neither holds this revision.

Verification: uv run --extra dev pytest tests/test_resume_remote.py tests/test_subagent_cli_remote.py tests/test_subagent_acp_remote.py tests/test_subagent_instance_registry_reload.py -q (139 passed); git diff --check github/main...HEAD passed.

…om its sheet (#1007) ## Summary Follows #1005, which is now on main; main was merged in after it landed, so the diff against main is this change alone. A registered machine whose node is up left the owner with nothing to do: the card said "Link up, no agent" and the Agents tab did not mention the machine. The way to connect an agent on it (the preset sheet's *Runs on*) is hidden exactly where it is needed most: a preset already connected on this computer is withheld from the roster, and a preset not installed here shows only install instructions. Measured on the owner's own setup (2026-10-19): two machines registered from the page, both reached and with the node, and no path on the page to put Claude Code on either. What changes: - `machines.list` rows carry `agents_here`, the connected agent entries that run on the machine (`agents_on_machine(only_enabled=True)`; a malformed agents section lists none). The card says "N connected here: ..." or "No agent connected here yet"; the state line now reads "Link up, no agent running", so "running now" and "connected here" are two lines. - A ready machine's sheet says the next step and offers **Connect an agent on this machine**: a form in the same drawer with the preset, its working directory there, and Connect. It calls the existing `subagents.add` with `machine`, `remote_cwd` and the name fixed to `<preset name>@<machine id>`, so the proof ping and every refusal are the add's own. - New read-only `subagents.presets`: the presets a machine's node can start (acp, cli), listed whether or not an entry already holds them, with `connected_here` and `on_machines` so the form greys the one this machine has and says when this computer has it too. Built-in agents, Mirasim shims and openai endpoints are not listed. - The Agents tab's "not installed" sheet also offers *Runs on* when ssh machines are registered, with a Connect button once a machine is picked: running on a machine does not need the agent on this computer. - New read-only `machines.local_keys`: the private keys this computer has, as `~/.ssh/<name>` (ssh's default identities in ssh's order, then any file with a `.pub` beside it). The register form fills the key path with the first and offers the rest, instead of a placeholder naming `id_ed25519` to an owner whose key is `id_rsa`; with none found it says so and points at the password login. - One preset once per machine, held by the server whichever way the add arrives: `subagents.add` without a `name` now refuses a preset the machine already has (judged on preset + machine, before the ping), naming the entry there and whether it is connected or switched off; a named add is a second entry on purpose (two working directories, say). The page's connect form leaves the name to the server's default (`<preset>@<machine id>`) rather than fixing its own spelling, so the page and a chat can no longer put one agent on one machine twice between them (the owner's setup had the two routes spelling the default differently, 2026-10-19). Contract (`rpc-schema/openrpc.json`), models, both generated clients and `i18n/messages.json` are updated; the offline fixture library answers the two new methods and the new field, and refuses the duplicate the way the server does. Docs: the machines guide (en + zh), CHANGELOG, CONTEXT. **Fifth round, after the review draft on LivXue's board (four findings, all reproduced) and the bot's inline note.** `machines.list` read the agents section for every row and caught only the pydantic error, so a `config.json` that does not parse at all (a `//` comment, a trailing comma) failed the whole listing with `internal_error` while the gateway ran on: `_agents_here` now treats the loader's `ConfigReadError` the same way (nothing listed; the agents page says why), and the test is a real unparseable file. The register form proposed `id_ed25519` before `id_rsa` and called that ssh's order; `ssh -G` lists `id_rsa` first on OpenSSH 8.9, 9.6 and 9.9, so an owner with both was proposed the key ssh itself tries second: `DEFAULT_IDENTITIES` follows the measured order, a test pins it against the ssh of the computer running it, and the guide's two sentences say so. A row that exists, switched off, with its command missing here, was offered **Connect on <machine>** from its sheet while the press sent the row's own `toggle`, which carries no machine: only a preset not yet on the roster is offered one, as the other *Runs on* already did. An add still running on a machine was unknown to a form opened again on that machine, so the preset could be pressed twice (a second proof prompt, then the duplicate refusal, a toast saying connected and a form saying not): the adds in flight are kept by the name each lands under (`mconnectPending`) and the form greys that preset (*being connected on this machine right now*). The contract's `subagents.presets` description names the add's default `<preset>@<machine id>` now, as the bot noted. The branch also carries #1005's fourth commit (merged) and main up to #1001. **Sixth round: merged onto #1005's head (`9dbef0874`) and LivXue's F1 fixed.** The merge had three conflicts, all the same shape -- this PR's addition beside what #1005 changed (the `auth` field in the machine sheet, `agents_here` in the fixtures, the new register assertions) -- and keeps both sides; the superseded password-used-once code (`generate_key`, `install_public_key`, `revoke_public_key`, `forget_key`, the remote scripts) is gone from the diff, so a review of this PR no longer grades that design. F1: a connect form open across a write reads `presets` again when the add lands -- whichever opening it is -- so a preset just connected on that machine reads as taken instead of inviting a second add the server refuses; the case that pinned the wrong end state moved with it. F2 (four ui-web files reformatted to the root prettier style) is withdrawn in 1834c68: each file is the #1005 text with this PR's own hunks replayed onto it (a three-way merge of the base, the base formatted the way the reformat had been, and this branch's head; ten overlaps resolved by hand, the maintainer edits' lines included), so what remains in them is this PR's change -- the four files' lines against #1005 go from 173/211/281/1694 to 137/44/206/590. Check used: formatting the result with the root config reproduces the previous head except where that head itself kept hand style, so nothing but wrapping changed. Verification, sixth round: related Python `1068 passed` over the twelve files this PR touches or affects, diff coverage 96.13% (348/362 executable changed lines); full suite `-n 6` 33386 passed, 3 failed -- the LibreOffice case and the two simulation cases this Mac always fails (run before the F1 commit, whose change is ui-web only); ui-web `vitest run` 4121 passed (250 files, gates included), `tsc --noEmit` and `eslint` clean, `gen:check` 247 methods; ui-tui `lint:i18n` and `lint:rpc` clean; the three range gates over `origin/main..HEAD`. **Also merged: #1005's c0c8222** (Oncall campaigns run on a password machine through trunk's askpass; the helper answers a password prompt only; `--password-stdin` implies `--auth password`) and the three maintainer edits on this branch (68ea011, e5884dd, dac563a), all clean. **Seventh round: two findings from the review of 91952ba.** (1) A configless Raven held a kept machine password unscrubbed: `held_secrets` returned early when `config.json` was missing, before it read `keys/<id>.password` -- and those live under the Raven home independently of any config, so a tool printing that file passed the password to the model verbatim (gloryfromca, inline). An absent config is now an empty config-credential set rather than the end of the call; the case is pinned with no config file at all, and the existing "no config holds nothing to scrub" case still holds for the config's own credentials. (2) CI shard 3/4 failed on `tests/test_node_stderr_replay.py::test_a_reason_written_on_stderr_while_the_link_was_down_reaches_the_run_watching_it` ("did not happen in time", then a leftover agent in its teardown) and `tests/test_subagent_cli_remote.py::test_...` -- both stand-in-machine timing cases this branch does not touch, the same class as the ones recorded for #965/#979/#983; 135 passed over those three files locally in one run, and the job passes on rerun. Verification: `tests/test_held_secrets_machine_passwords.py` and `tests/test_security_untrusted_context.py` 42 passed; the other three files that read the scrubber 223 passed; `ruff check`, `ruff format --check` and `ty check` clean on the two changed files; the three range gates over `origin/main..HEAD`. ## Type - [ ] Fix - [x] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification - `pytest tests/test_rpc_machines.py tests/test_ops_connection_add.py tests/test_rpc_subagents.py tests/test_rpc_schema_match.py tests/test_rpc_registration.py -n 4`: 768 passed (7 new: `agents_here` incl. the malformed-section case, `local_keys` order and exclusions at both layers, `subagents.presets` membership and placement). - Full suite `pytest -n 6`: see the last comment on this PR for the count (run at the head). - `ruff check` / `ruff format --check` on the changed Python files: clean. - Duplicate-on-machine refusal: 2 new cases in `tests/test_rpc_subagents.py` (default-named second add refused before any ping, naming the entry and its state; named add allowed; another machine allowed; switched-off entry named), both fail with the handler change stashed; related groups 928 passed after it. - ui-web: `vitest run` 4006 passed (250 files, page gates included; 7 new page cases: the card line and the sheet's next step, the connect form end to end incl. a refusal and the nothing-to-connect case, key prefill with and without keys, an absent agent connected onto a machine from its own sheet, and no machine offered when none is registered); `tsc --noEmit` clean; `eslint` clean on the changed files; `gen:check` matches the contract. - ui-tui: `gen:rpc` and `gen:i18n` regenerated; `pytest tests/test_docs_site.py` 16 passed; `mkdocs build --strict` clean. - Real machine, isolated gateway (own Raven home and registry copy, channels off, the branch's node installed in its own directory on the test machine), real browser: the ready machine's sheet shows the next step and the button; the form lists 12 startable presets; Connect of Claude Code on the machine landed in 9 s and the card then reads "1 connected here: Claude Code@<id>"; the register form was prefilled with this computer's `~/.ssh/id_ed25519` with `id_rsa` as the other choice; an agent absent here (OpenClaw) connected onto the machine from its own sheet was refused in 1 s with the agent's own sentence ("the agent's command was not found on machine ...") shown in place. Gateway stopped by pid, the node directory and daemon on the machine removed afterwards. - Previews of the five surfaces were shown to the owner before the change and approved. - Fifth round, red before and green after: `machines.list` over a real `config.json` that is not JSON lists the machine with no agent (`test_list_lists_no_agent_over_a_config_file_that_does_not_parse_rather_than_failing`; through the real `Dispatcher` with a scratch home and registry, the previous handler answered `-32603 internal_error`, this one `[('box', [])]`); `local_keys` order at both layers, plus `test_the_default_identities_are_in_the_order_this_computer_s_ssh_offers_them` against `ssh -G -F /dev/null` (this Mac, OpenSSH 9.9: `id_rsa, id_ecdsa, id_ecdsa_sk, id_ed25519, id_ed25519_sk, id_xmss, id_dsa`; the same on a registered Linux machine, OpenSSH 9.6); the page: a configured, switched-off, missing row gets no machine field and no *Connect on* (`offers a row that exists no machine...`), and a form opened again on a machine while an add still runs there greys the preset, keeps the other one pressable and sends no second add (`greys a preset whose add on the machine is still running...`) -- both fail with the three page sources stashed. The six Python files 821 passed, diff coverage of the changed lines 97.50%; ui-web `vitest run` 4119 passed (page gates 216 included), `tsc`, `eslint` on the feature, `gen:check` (247 methods), `npm run build` with both boot goldens; ui-tui `gen:rpc`/`gen:i18n`, `lint:i18n -- --check`; `ty check` on the changed modules; `pytest tests/test_docs_site.py tests/test_living_docs.py` 18 passed, `mkdocs build --strict` clean; the three range gates; full suite (`-n 6`) 33353 passed, 4 failed: the three this Mac always fails (a LibreOffice case, two simulation cases) and one stand-in-machine timing case in `tests/test_subagent_cli_remote.py` (the record not yet written when the test reads it) that passes 3 of 3 runs alone and touches no file this branch changes. ## Risk - [x] Security impact considered: `machines.local_keys` returns key *paths* on this computer, to this computer's own page, never key material; `subagents.presets` carries no credential; nothing new leaves the host. The connect form reaches the machine only through the existing `subagents.add` and its proof ping. - [x] Backward compatibility considered: `agents_here` is a new required field on `machines.list` rows (the page maps it defensively); the two new methods are additive; an older page against this server ignores both. - [x] Rollback path is clear for risky changes: revert the commit; no stored data changes shape. ## Related Issues N/A (follow-up to #983 and #1005, from the owner's walkthrough on 2026-10-19) --------- Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com> Co-authored-by: Dizhan Xue <xuedizhan17@mails.ucas.ac.cn> Co-authored-by: MiniMax (minimax-m3.1-flash-free) <noreply@minimax.io>

Handsome-wzw requested a review from LivXue as a code owner October 16, 2026 03:43

Handsome-wzw mentioned this pull request Oct 16, 2026

fix(*): machine runs outlive a killed daemon, a refused failure announce, a dead-link cancel #957

Merged

7 of 12 tasks