Repository navigation
fix(node): index off command lines, a bad port in one sentence, the node's own version - #963
Conversation
…ode's own version Three fixes to installing a node, the first two from the review of EverMind-AI#920. The package index pip is configured with on the machine went to uv as --default-index on the step's command line. pip takes a sign-in inside that URL, and the line was in this computer's ssh argv, the machine's sh -c argv and uv's argv, which every account on the machine can read in /proc/<pid>/cmdline. The index now travels on the stdin of the two steps that run uv pip into UV_DEFAULT_INDEX, the variable uv reads as --default-index; a step that finds no line there fails rather than install from PyPI behind the owner's back. A port in that URL that is not a number from 0 to 65535 made the sign-in-free display raise ValueError, which install-node does not catch, so the person saw a traceback. The display now cuts the sign-in from the text after the authority's last @ and never reads .port, and the look at the machine refuses such an index in one sentence before anything is installed. An empty port is the scheme's default and goes ahead. A node runs this code from lib/, not as an installed distribution, so it read no version of its own: its socket, lock and pid under run/, and the version it reports at the handshake, said 0.0.0+unknown while its directory is named by the host's version. The install now writes a distribution record beside the code with the version the directory is named by, outside lib/raven, so the digest is the same. Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the full diff against github/main, the install runner and SSH stdin transport, version discovery and node naming callers, relevant history, and the Raven node architecture contract in CONTEXT.md. I also checked the repository rules, backward compatibility, and that tests were extended rather than weakened. The index stays out of local, remote-shell, and uv argv; invalid ports produce a credential-free InstallError; and the added dist-info record supplies the host version without changing the bundled-code digest.
Verification: uv run pytest tests/test_node_install.py tests/test_cli_ops_connection_commands.py tests/test_node_bundle.py tests/test_node_client.py tests/test_node_server.py tests/test_node_relay.py tests/test_subagent_acp_remote.py tests/test_import_cycle_budget.py -n 6 -q passed with 334 tests after uv sync --all-packages. git diff --check github/main...HEAD also passed. The first test attempt did not execute test bodies because the fresh environment lacked the declared workspace package providing raven_everos; syncing all workspace packages resolved that setup issue.
|
No blockers in the reviewed scope. Scope: I reviewed only the What I examined: the three changes to Observations outside the reviewed scope:
Verification:
|
…nce, a dead-link cancel (#957) ## Summary Seven edges of a run on a registered machine, left by the review of #934 and by timing probes on the real machine. Each is a fix the host or the node makes on its own; nothing changes for an entry without `machine`. - **A Raven node killed outright no longer leaves agents behind or a bare `exit -1`.** The next daemon brings every record left at `running` to `lost` at its start (`Daemon.recover`), ends the agent when its pid is still the process it was started as (group leader, the recorded command, and the start mark the launch recorded: Linux's `/proc` start tick, elsewhere `ps -o lstart`), and writes why; a record without the mark, or a pid whose mark differs, is left alone and recorded as gone. `agent/exit` and the attach answer carry that `reason`; the host hands it on in place of the exit code (`ChannelEndpoint.exited(rc, reason)`), so a cli run says "the Raven node on this machine was restarted while the agent ran, and the agent is gone" and a picked-up acp run says the same. On Linux the agent is also tied to the daemon's life (`PR_SET_PDEATHSIG`, a SIGTERM). Protocol number unchanged: the field is optional and old nodes never send it. - **A failed remote run's error survives a draining-refused announce.** The manager's two error branches read whether the announce was taken, as the reply branch does (`announced = taken is not False`), and the acp backend keeps the record of a prompt that went out and failed, the cli backend that of a timed-out command, for the caller that reports it. The next start reads how the task ended off the machine and announces the error. - **A cli run picked up after a restart binds its instance to the session id in its output** (`_bind_picked_up`), as the dispatch's deferred commit would have; with no id in the output the reply carries the same "not resumable" warning the dispatch would. A `provisioned` id or a stateless run binds nothing. - **A cancel while the machine's link is known dead returns at once.** `_cancel_turn` reads `ChannelEndpoint.offline` (parked, or the link not alive), marks the session unsettled and leaves the held `session/cancel` for the next link, instead of waiting the settle budget for a settlement that cannot arrive. In the gap before the heartbeat notices nothing changes. - **An openai row's tunnel gives up a taken local port for another.** The port is checked again just before ssh starts (`_still_free`), and ssh's own "Address already in use" / "Could not request local forwarding" ends in another try (`_TUNNEL_TRIES` = 3) rather than in a request sent to whoever took the port; each attempt reads only what ssh wrote in its own run of the log. - **A run record that is not JSON, or not UTF-8, is set aside once** (`<name>.json.unreadable-<ts>`), named in the log that once, instead of being warned about at every start; a record a later version wrote is left alone as before. (The first revision read the record as text and let a `UnicodeDecodeError` out of the whole scan, found by the Windows review; the record is read as bytes now and decoded by `json.loads`, so a decoding failure is set aside like a JSON one, and one bad record never stops the others from being read.) - **A path written as remote that names no registered machine** is refused in one sentence for `write_file` and `edit_file` too, never taken for a relative path here. This has been the case since the debug switch went (#934); a regression test pins it. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification Every fix has a test that failed on the old code first. - `.venv/bin/python -m pytest -n 6 -q tests/test_node_server.py tests/test_node_client.py tests/test_remote_runs.py tests/test_subagent_manager.py tests/test_subagent_cli_remote.py tests/test_remote_file_tools.py tests/test_subagent_acp_remote.py tests/test_resume_remote.py tests/test_remote_resume.py tests/test_subagent_acp.py` - all green on macOS (the Linux-only pact test skipped there); the last run of the four fast files 331 passed, cli remote 52, remote file tools 24, the acp remote recovery tests 3 - Full suite: 31577 passed, 234 skipped, 2 xfailed in 6 min 14 s; the 3 failures are the ones `main` has on this machine (a host without LibreOffice, two simulation records). Changed-line coverage: 98.18% of 165 executable changed lines (`scripts/coverage_gate.py diff --base-ref origin/main`); the 3 uncovered lines are the Linux-only `prctl` import block, which CI's Linux shards run. - `ruff check` / `ruff format --check`, `ty check` on the changed modules, `lint-imports` (10 contracts kept), `scripts/check_commit_file.py`, `scripts/check_source_language.py`, `scripts/check_large_files.py`: clean. - Real machine (ravenx2, an isolated registry copy and Raven home per tree; the node of each tree installed under its own `node_dir`; the same probe script on this branch and on `main`): | Probe | `main` | this branch | |---|---|---| | P1 `kill -9` the daemon under a running cli turn | 4.9 s after the kill: `CLI agent 'x' exited -1`; the loop the daemon had started was still running (1 left) | 5.1 s: `CLI agent 'x': the Raven node on this machine was restarted while the agent ran, and the agent is gone`; 0 left; a new daemon pid | | P2 an acp prompt that fails after it went out, `reported_by_caller` | the prompt failed (`exit 5`); no record left | record kept; the next start announced the error with `exit 5` and removed the record | | P3 cancel while the link is known dead; the cancel on the next link | the cancel returned after 2.35-2.46 s, held until the link was back (a 2 s backoff in the probe; up to the 5 s budget in production); it landed on the next link (task frames 3->4) | 0.00 s; landed the same way (3->4 within 0.1 s of the restore) | | P4 the chosen local tunnel port is taken by a listener | the tunnel used the taken port; the request timed out against the listener that held it | the next port; HTTP 200 through the machine | | P6 a derived-id cli create left running, picked up by the next start | picked up ok, no binding (`commits=[]`) | picked up ok, the handle bound to `edge-123` | After the merge of `main` (which brought #947's MCP streams, #953's host-death sweep and #963's install fixes into the same files): the related groups (node server/client/bridge, remote runs, resume, cli remote, acp remote, manager, remote file tools, acp client, import-cycle budget) 1006 passed; diff coverage 97.30% of 185 changed lines (the 5 uncovered are the Linux-only `prctl` lines, run by CI's Linux shards); the real-machine probes re-run on the merged tree: 8/8 PASS (P1 5.2 s, 0 orphans; P2 record kept and the error announced; P3 0.00 s and the cancel landed on the next link; P4 the next port, HTTP 200; P6 bound to `edge-123`). Docs checked against the code before pushing: CONTEXT.md's Raven node entry (records of failures, set-aside records, the daemon's recovery and the `reason`), the module docstrings of `remote.tunnel`, `remote_runs.pending`, `Daemon.recover`, `_still_ours`, `_death_pact`, `ChannelEndpoint.exited/offline`, `_bind_picked_up`, `_track_on_machine`, the `agent/exit` comment in `protocol.py`, and the CHANGELOG entry. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed ## Risk - [x] Security impact considered - the daemon ends only a pid that is still the process it started: group leader of its own group, running the recorded command, and started when the record says (`born`, written at the launch); a reused pid fails the last mark even for the same program and is left alone, the record then saying the agent is gone. No new network surface; the `reason` is the node's fixed sentence, never user or agent text. - [x] Backward compatibility considered - `agent/exit`'s `reason` is optional, protocol number unchanged; a host on this branch reads an old node's exits as before, an old host ignores the field. Records written before this branch read unchanged. - [x] Rollback path is clear for risky changes - a plain revert; nothing is migrated. ## Related Issues Follow-up to #934 (the two non-blocking notes of its review) and to the timing probes run on #921/#922/#934. --------- Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
…install's (#965) ## Summary Three follow-ups to #963 (installing a Raven node on a registered machine), from LivXue's review of it. Only `raven/node/install.py` and its test change. - **An old uv no longer skips the configured index.** #963 hands uv the index pip is configured with on the machine through `UV_DEFAULT_INDEX`, which uv reads since 0.4.23 (uv's changelog adds it there together with `--default-index`). A machine's own uv older than that ignores the variable and installed from PyPI while the plan named the mirror -- before #963 such a uv refused the `--default-index` argument instead, loudly. The probe now reports the found uv's version (`UV_VERSION`, from `uv --version`). With an index configured, a uv older than 0.4.23, or one that does not say its version, is passed over: the pinned uv is fetched under the node dir as for a machine with none, and the plan says why ("the machine has uv 0.4.22, and only uv 0.4.23 or later installs from the index pip is configured with"). With nothing to fetch it with, the install says so in one sentence naming the version. Without an index configured an old uv is used as before. - **The port check is an install's, not the look's.** #963 refused an index whose port is not a number from 0 to 65535 in `plan()`, which `install-node` calls before it reads whether the node is already there and before `--prune`; neither needs the index, so a machine that had its node and a bad port in pip's configuration could not be told so or pruned. The check is now in `refusal()`, with the same sentence; the look takes the machine as it is. - **The real install test no longer reaches the network.** Its `run` stand-in stood in for `uv pip install`; since #963 the two steps that run it go through `push` when an index is configured, and the `push` stand-in ran them for real -- on a computer with a system-wide `pip.conf` the test installed packages, against the module docstring's promise. The `push` stand-in now stands in for them too, and the test also runs with an index configured, checking both steps are fed it on stdin. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification Follow-up commit, after the review of this PR: - A uv an earlier install fetched under `node_dir/.uv` is used in place of a machine uv that would ignore the index. The probe reports that uv on its own (`NODE_UV`, `NODE_UV_VERSION`) beside the one it found first, and `plan()` swaps it in when the first is older than 0.4.23 (or silent) and the fetched one is not; without an index the machine's own uv serves as before, and one under `node_dir` that is itself too old does not help. Two tests failed first: the plan from such a probe (uses the fetched uv, no `uv` step, 60 MB, no refusal without curl), and the probe under a real `/bin/sh` reporting both uvs (then the one, then none). - The machines guide, English and Chinese: the "What lands on the machine" row for `uv` names the old-uv case, and the troubleshooting table has a row for the sentence an old uv earns. `tests/test_docs_site.py` and `tests/test_living_docs.py` pass (18), `mkdocs build --strict` passes. - The docstring of `test_a_plan_whose_index_port_cannot_be_read_still_describes_itself_and_its_failures` says the install, not the look, refuses the port. The real install test's `uv_version` is pinned to `UV_VERSION` rather than read off this computer's uv, so a test host with a uv older than 0.4.23 does not reach astral.sh through the stand-in that stubs only `pip install`. - `tests/test_node_install.py` -- 82 passed; with the cli, bundle, client and import-cycle files -- 204 passed; `.venv/bin/python -m pytest -n 6` -- 31781 passed, 3 failed (the libreoffice and two simulation cases, which fail the same way on main on this computer); diff coverage against main 100% (30/30 changed executable lines). - Real machine (a registered Linux machine; everything under this check's own directory, removed after): uv 0.4.22 from uv's release page was put first on the PATH the check's own runner gives the probe, with an index configured. Before this commit: the first install passed it over and fetched uv 0.13.0 under `node_dir` (85 MB planned), and the second look found 0.4.22 first again, planned to fetch 0.13.0 again (85 MB, an `uv` step), and with curl and wget taken away was refused with "upgrade uv there" while 0.13.0 sat under `node_dir`. After: the same first install, then the second look found the fetched 0.13.0, planned 60 MB with no `uv` step, and went ahead without curl or wget. - `.venv/bin/python -m pytest tests/test_node_install.py` -- 80 passed. Each new or changed case failed on main first: with an index configured, the real install test's packages step really ran `uv pip install` against the test's mirror and failed resolving it (now stood in for, both steps fed the index on stdin); `plan()` of a machine whose index names a port that is not a port now succeeds, `refusal()` gives the sentence, `install()` runs nothing, and a machine that has this node is found so and pruned; for a uv at 0.4.23 and later, older, and silent about its version, with and without curl or wget, the plan uses the machine's uv or fetches the pinned one and says why, or `refusal()` gives the exact sentence; an old uv still serves a machine with no index; under a real `/bin/sh` the probe reports the version a stand-in uv prints and nothing for one that prints none. - `.venv/bin/python -m pytest tests/test_node_install.py tests/test_cli_ops_connection_commands.py tests/test_node_bundle.py tests/test_node_client.py tests/test_import_cycle_budget.py -n 6` -- 202 passed. - `.venv/bin/python -m pytest -n 6` -- 31780 passed, 3 failed (`test_rpc_files.py::test_a_host_without_libreoffice_says_so` and the two simulation cases, which fail the same way on main on this computer); diff coverage against main 100% (26/26 changed executable lines). - `ruff check`, `ruff format --check`, `ty check raven/node/install.py` clean; `check_commit_messages.py`, `check_source_language.py` and `check_large_files.py` on `origin/main..HEAD` clean. - Real machine (a registered Linux machine whose pip is configured with an HTTPS mirror; this check's own node dir, removed after): uv 0.4.22 from uv's release page was put where the probe finds a uv, and the install was given an index pointed at a port nothing listens on, so whether uv used it shows. With main's code the old uv was used and the install succeeded -- uv went to PyPI behind the plan. With this change the probe read 0.4.22, the plan fetched uv 0.13.0 and said why, the packages step failed with "Connection refused" (the index was used), and the node dir held uv 0.13.0 after. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed The install module's docstring now says an old uv with an index configured is passed over; the new `Plan` field and the moved check are documented where they are. `CONTEXT.md` (the node entry) does not describe uv versions or the index and still holds. ## Risk A machine whose own uv is older than 0.4.23 and whose pip is configured with an index now gets the pinned uv fetched under the node dir (about 25 MB, as for a machine with no uv), or one sentence asking to upgrade uv when it has no curl or wget; every other machine is unchanged. A machine with a bad port in pip's configuration can again be told it has the node and be pruned; an install there is still refused with the same sentence. The change is to the install plan and one probe line; rollback is a plain revert. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues Follows the review of #963 (the observations F1, F2 and F3 left on it). --------- Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
Summary
Three fixes to installing a Raven node on a registered machine
(
raven ops connection install-node), all inraven/node/install.py. Thefirst two are nonblocking notes LivXue left on #920; the third is an older
problem with the version a node names itself by.
package index pip is configured with on the machine, and the install
handed it to uv as
--default-index <url>inside the step's command. piptakes a sign-in inside that URL (
https://user:password@host/simple), andthe command was in this computer's ssh argv, the machine's
sh -cargv anduv's argv;
/proc/<pid>/cmdlinethere is readable by every account on themachine. The index now travels on the stdin of the two steps that run
uv pip install(packages,search) and is read intoUV_DEFAULT_INDEX,the variable uv reads as
--default-index; a process's environment isreadable only by its own user. A step that finds no line on stdin fails
rather than install from PyPI behind the owner's back. With no index
configured nothing changes: no stdin, uv's default.
_shown_index, which drops the sign-in from the URL for every sentence,read
urlsplit(url).port, which raisesValueErrorfor a port that is nota number or is past 65535;
install-nodecatches onlyInstallError, sothe person saw a traceback. The display now cuts the sign-in from the text
after the authority's last
@(asurlsplitfinds it) and never reads.port, and the look at the machine refuses such an index before anythingis installed: "the package index pip is configured with on machine ... has
a port that is not a number from 0 to 65535 ();
correct index-url in pip's configuration there, or PIP_INDEX_URL". An empty
port is the scheme's default (RFC 3986) and goes ahead.
lib/, not asan installed distribution, so
importlib.metadatafound noraventhere:raven.__version__fell back to0.0.0+unknown, and with it the socket,lock and pid the daemon keeps under
run/(bundle.install_name()) andthe version the node reports at the handshake, which the machine listing
shows -- while the node's directory is named by the host's version. The
pathstep now also writes a distribution record,lib/raven-<version>.dist-info/METADATA, with the version the directoryis named by, so the node reads the same version and names itself as the
host named it. It sits outside
lib/raven, so the digest is unchanged.This needs no change to
raven/node/server.pyorraven/node/bundle.py.A node installed before this keeps its old names until it is reinstalled;
any change to the code changes its digest and asks for a reinstall anyway.
Type
Verification
.venv/bin/python -m pytest tests/test_node_install.py-- 69 passed. Eachnew or changed case failed on main first (12 failed there): no step
command names the index, its host or its sign-in, and
install()feeds thewhole URL as one line on stdin to exactly the
packagesandsearchsteps (the
codestep still gets the tarball); under a real/bin/shwitha stand-in uv that records its argv and its
UV_DEFAULT_INDEX, the packagesstep and both tries of the search package (the machine's index, then beside
the public one) see the index in their environment and never in argv, and
with nothing on stdin the packages step fails before uv runs;
_shown_indexdrops the sign-in for a port that is not a number, one past65535, an
@inside the password and an IPv6 host, without raising;plan()refuses an index whose port is not a number or is past 65535 withthe exact sentence above and runs nothing after the look, while a numeric
port, no port and an empty port go ahead; a plan holding such an index
still describes itself and its failures; the
pathstep passes theversion the install name begins with; and the one real install on this
computer (a uv virtualenv of this interpreter, this code unpacked into it)
now has the node report
raven.__version__andbundle.install_name()equal to this computer's install name under
-I..venv/bin/python -m pytest tests/test_node_install.py tests/test_cli_ops_connection_commands.py tests/test_node_bundle.py tests/test_node_client.py tests/test_node_server.py tests/test_node_relay.py tests/test_subagent_acp_remote.py tests/test_import_cycle_budget.py -n 6-- 334 passed.
scripts/coverage_gate.py diff --base-ref origin/main --threshold 90--100% (16/16 changed executable lines).
ty check raven/node/install.py,ruff checkandruff format --checkclean;check_commit_messages.py,check_source_language.pyandcheck_large_files.pyonorigin/main..HEADclean..venv/bin/python -m pytest -n 6on this tree -- 31716 passed, 172 skipped,2 xfailed, 3 failed:
test_rpc_files.py::test_a_host_without_libreoffice_says_so,test_simulation_scenario.py::test_a_fresh_trial_refreshes_the_subagent_homes_before_it_runsand
test_simulation_suite.py::test_a_stopped_run_is_marked_in_its_record_and_finished_with_its_evidence,which fail the same way on main on this machine.
Real machine (a registered Linux machine whose pip is configured with an
HTTPS mirror, installed into a node directory of this check's own and
removed after): the machine's mirror URL was given a made-up sign-in, a node
was installed, and the process lines on the machine and on this computer
were sampled every 0.3 s while it ran. With main's code: the sign-in was in
a process line in 22 of 46 samples, in
shanduvon the machine and insshhere; the node reported version0.0.0+unknownat the handshake andits
run/held0.0.0+unknown-<digest>.lock,.pidand.sock. Withthis change: 0 of 68 samples; the node installed, answered the handshake
with this code's digest and version
0.2.4, itsrun/held0.2.4-<digest>.*, and its ownraven.__version__and install name matchedits directory. With the index pointed at a port nothing listens on, the
install failed at the
packagesstep with "Connection refused" -- uv tookthe index from its environment and did not fall back to PyPI -- and the
sentence held no sign-in. Nothing of this check was left on the machine.
Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
No user-facing text changes beyond the new refusal sentence; the install's
module docstring and the
Plan.indexfield say where the index travels andwhat the version record is for.
CONTEXT.md(the node is installed "in adirectory named by the code's version and digest") was checked and still
holds.
Risk
The index now reaches uv only through its environment, which only the
machine user can read; nothing else about the install changes. uv has read
UV_DEFAULT_INDEXsince it gained--default-index, so a machine's uvthat understood the flag understands the variable. A machine whose pip
configuration names a port that is not a number now gets one sentence
instead of an install that would fail at the packages step. The version
record is a few lines beside the code, outside the digested package, so a
node of this code reports the same digest. Rollback is a plain revert; a node
installed with the record keeps working with older code, which does not read
it.
Related Issues
Follows the nonblocking notes on #920 (the install's package index on the command line, and the traceback for a port that is not a number).