From ac98cb20db436b49625d6b8c32bd09b236c6d725 Mon Sep 17 00:00:00 2001 From: "xiaotian.luo" Date: Wed, 23 Sep 2026 11:52:10 +0800 Subject: [PATCH 1/6] refactor(*): one registry reader and one writer, and the coding agent can add a machine Two products run work on the owner's machines, and in the usual chain the coding agent runs first: it builds a case where the solver is and smoke-tests it there, then hands off to the on-call agent. Yesterday's run showed the seam between them was broken twice over. Read side. `raven.ops.connections.store_path()` resolved the registry "beside the config". A sub-agent runs on a rendered config in a state directory of its own, so Raven-Code read an empty directory while the owner's seven machines sat one level up in the home the host had handed it as RAVEN_HOME; `exec(machine=...)` answered "No connection is registered", and the model reached for the raw ssh address instead. The path now resolves to the env var, else the owner's home when a registry is there, else beside the config when THAT exists, else the home -- which is also where a first add lands, so what the coding agent writes is what the on-call agent reads. The on-call launcher's RAVEN_CONNECTIONS pointer keeps working and is no longer the only thing that does. Write side. The tool that takes the owner's answer and writes a machine (`ops_connection_add`, #545) lived in the on-call plugin; the coding agent, which meets the empty registry first, had no way to consume the answer. The probe, the writer and the ssh alias now live in `raven.ops.connection_add`, behind both the tool (`raven/agent/tools/connection_add.py`) and `raven ops connection add`; the plugin's copies are gone. Key isolation for a candidate the tool picked itself moved with it, as `raven.ops.transport.make_ssh_runner(identities_only=)`, and the probe rides the same transport `exec`'s machine channel uses. The tool is opt-in per product (`tools.connectionAdd`): on in raven-code and raven-oncall, off elsewhere, because five product tool faces are pinned and a lane that never runs work on the owner's machines has no use for a tool that writes to their ssh config. The on-call plugin's `connections` module kept a hand-aligned copy of trunk's reader (eleven functions) on the ground that trunk's module was not a contracts paper; the plugin already imports trunk's utilities and hooks, and a third copy for the coding agent was the alternative. It imports the reader now and keeps only what a campaign needs (lookup by id, display name, meta resolution, the model-facing listing). The ask the model relays to the owner is one text (`ASK_OWNER`), shared by the on-call listing reply and the tool. Raven-Code's guide gains the rule for leaving this computer at all: write here; verify where the software is; ask the owner for a machine only when installing the software here would be wrong (GPU, memory) or heavy (a compiled solver, a toolchain, a long download); never type an ssh from an address in the task statement. Tests: the connection-add suite moves to trunk and patches the kernel modules; the plugin's two "aligned by hand" parity tests become one identity pin; five new cases pin the path resolution; the code product's face ledger gains a machine lane. Complements #552 (exec refuses a typed ssh to a machine the registry knows), which only fires once the registry is visible. Co-authored-by: Claude (claude-opus-5-5) --- CHANGELOG.md | 21 + agents/raven-code/config.json | 1 + .../plugins/code-flow/prompts/TOOLS_CODE.md | 26 + agents/raven-oncall/config.json | 1 + .../oncall-flow/oncall_flow/connections.py | 637 ++---------------- .../oncall-flow/oncall_flow/docker_backend.py | 27 +- .../oncall-flow/oncall_flow/tools/__init__.py | 8 - .../plugins/oncall-flow/raven-plugin.toml | 7 +- docs-site/docs/oncall.md | 22 +- docs-site/docs/oncall.zh.md | 17 +- raven/agent/loop/_shared.py | 1 + raven/agent/loop/bundles.py | 3 + raven/agent/loop/main.py | 2 + raven/agent/loop/wiring.py | 8 + .../agent/tools/connection_add.py | 90 +-- raven/cli/ops_connection_commands.py | 175 +---- raven/config/schema.py | 6 + raven/core/runtime.py | 1 + raven/ops/connection_add.py | 243 +++++++ raven/ops/connections.py | 56 +- raven/ops/transport.py | 25 + tests/test_agents_code_launcher.py | 22 +- tests/test_agents_oncall_flow_connections.py | 85 ++- tests/test_agents_oncall_flow_watcher.py | 1 - tests/test_agents_oncall_launcher.py | 15 +- ...tion_add.py => test_ops_connection_add.py} | 93 ++- tests/test_ops_connections.py | 77 +++ 27 files changed, 732 insertions(+), 938 deletions(-) rename agents/raven-oncall/plugins/oncall-flow/oncall_flow/tools/ops_connection_add.py => raven/agent/tools/connection_add.py (70%) create mode 100644 raven/ops/connection_add.py rename tests/{test_agents_oncall_flow_connection_add.py => test_ops_connection_add.py} (65%) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab333a882..80626e961 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,18 @@ All notable changes to Raven are documented here. ### Added +- `ops_connection_add` is a trunk tool, served on `tools.connectionAdd` + (on in raven-code and raven-oncall, off elsewhere). It writes the machine + an owner described into their connection registry after reaching it, with + `ssh -G` filling a port, user or key they left out and each candidate key + probed on its own (`raven.ops.transport.make_ssh_runner(identities_only=)`). + The probe, the writer and the `~/.ssh/config` alias live in + `raven.ops.connection_add`, behind both this tool and `raven ops connection + add`; the on-call plugin's copies are gone. Raven-Code's guide gains the + rule for when to leave this computer at all: write here, verify where the + software is, and ask the owner for a machine only when installing the + software here would be wrong or heavy. + - Serply joins the `web_search` vendors: `tools.web.search.provider: serply` with the key under `tools.web.providers.serply.apiKey` (or `SERPLY_API_KEY`). Google SERP rows normalised into the shared render path; the research @@ -22,6 +34,15 @@ All notable changes to Raven are documented here. ### Changed +- The connection registry resolves to the owner's home first + (`raven_home()/connections.json`), then beside the instance's own config, + then the home again for a first write. A sub-agent runs on a rendered + config in a state directory of its own and is handed `RAVEN_HOME`, not a + copy of the file; resolved beside the config alone, Raven-Code read an + empty directory while the owner's machines sat one level up, and reached + for the raw ssh address instead. The on-call plugin's `connections` module + now imports trunk's reader rather than mirroring it. + - Progressive tool disclosure ships on (`tools.toolSearch.enabled`). Below `compactionThreshold` (50) nothing changes: the strategy drops `tool_search` from every request while the catalog fits. Above it, most tool schemas are diff --git a/agents/raven-code/config.json b/agents/raven-code/config.json index e869b1a17..b030b7931 100644 --- a/agents/raven-code/config.json +++ b/agents/raven-code/config.json @@ -30,6 +30,7 @@ "exec": { "timeout": 600 }, + "connectionAdd": true, "web": { "search": { "maxResults": 5 diff --git a/agents/raven-code/plugins/code-flow/prompts/TOOLS_CODE.md b/agents/raven-code/plugins/code-flow/prompts/TOOLS_CODE.md index a2c1e7398..f2d716c7a 100644 --- a/agents/raven-code/plugins/code-flow/prompts/TOOLS_CODE.md +++ b/agents/raven-code/plugins/code-flow/prompts/TOOLS_CODE.md @@ -33,6 +33,32 @@ the host exec instead; follow that tool's stated limits and failure behavior. the timeout ceiling; poll its log with `read_file`. Stop the ones that were only scaffolding before you finish +## exec on another machine — when, and how a machine gets listed + +- Write here when you can. A case, a script, an input deck: this computer + writes them. Go to another machine only to *verify* — one short run where + the software is — and only when installing that software here is the wrong + move: it needs a GPU or memory this box has not (installed here it would run + nowhere), or it is heavy to install (a compiled solver, a system toolchain, + a download that outlasts the verification itself). A small library that + installs in seconds is installed here, and nobody is asked. +- Before building, name what the task needs to run — GPU, solver, library, + memory. If this box lacks it and installing it here is wrong, look at the + machines the owner listed: `exec` takes a `machine` id when any is listed, + and an unknown id answers with the list and what each machine has installed. + Pick the one that has what the task needs, and say which and why. +- When none fits, or none is listed at all, ask the owner what + `ops_connection_add` says to ask — this computer or another, what they call + it, an address if another — and hand their answer to that tool. It reaches + the machine before writing anything; ssh's own config fills a port, user or + key they left out. The owner is asked once: the machine it writes is the + one the on-call agent reads afterwards. +- Never reach a machine by typing an `ssh user@address` from a task statement + or a config file, and never put a key path or an address into a command. + The way onto a machine belongs to its registry row, below you; `machine=` + is how you use it. Could not verify on the machine? Say so in the hand-off, + so the first on-call round is a smoke test and not a full run. + ## read_file / write_file / edit_file — paging, reading before editing - `read_file` returns 2000 lines per call by default. A large file is NOT fully diff --git a/agents/raven-oncall/config.json b/agents/raven-oncall/config.json index e10c8eab7..e6baece92 100644 --- a/agents/raven-oncall/config.json +++ b/agents/raven-oncall/config.json @@ -29,6 +29,7 @@ "timeout": 600, "maxTimeout": 1200 }, + "connectionAdd": true, "web": { "search": { "maxResults": 5 diff --git a/agents/raven-oncall/plugins/oncall-flow/oncall_flow/connections.py b/agents/raven-oncall/plugins/oncall-flow/oncall_flow/connections.py index 0aa41fca0..d0ca730ae 100644 --- a/agents/raven-oncall/plugins/oncall-flow/oncall_flow/connections.py +++ b/agents/raven-oncall/plugins/oncall-flow/oncall_flow/connections.py @@ -26,265 +26,50 @@ one means a keychain or a master password, and neither is worth building before something asks for it. -The file is written by hand, by the host's ``raven ops connection add``, or -- -since 2026-09-20 -- by this instance's own ``ops_connection_add`` once the owner -has answered what that command asks (the host stopped gating a spawn on the -registry on 2026-09-06, and with it went the step that ran the -command on the owner's behalf; the ask survived, the write did not). This module -is the plugin's own reader and writer over the same store the trunk module -reads (RAVEN_CONNECTIONS, else the owner's ``~/.raven/connections.json``): the -trunk module is deliberately not a contracts paper, so the plugin cannot import -it at runtime -- behaviour is aligned to the trunk version and any drift is a -parity-ledger entry (C2). ``probe``, ``write`` and ``write_ssh_alias`` mirror -``raven/cli/ops_connection_commands.py`` for the same reason. +Trunk's :mod:`raven.ops.connections` is the reader and +:mod:`raven.ops.connection_add` the writer; both are imported here rather than +mirrored. This module kept a byte-aligned copy of the reader from the vendored +fork until 2026-09-23, on the ground that trunk's module was not a contracts +paper; the plugin already imported trunk's utilities and hooks, the copy had to +be re-aligned by hand on every change, and the coding agent came to need the +same functions -- so a third copy was the alternative. What stays here is what +only a campaign needs: looking a connection up by id, its owner-facing name, +filling a campaign's meta from its row, and the listing the on-call model reads. """ from __future__ import annotations -import json -import os -import re -from dataclasses import dataclass -from pathlib import Path from typing import Any -STORE = "connections.json" - -SSH, LOCAL = "ssh", "local" - -# Without these there is no way onto the machine, so a row missing one cannot be -# used for anything and saying so is a refusal. The list is short on purpose: -# every row that existed before this file was written predates most of the fields -# below, and those rows work. A check that retroactively condemns a working -# registry is a check the owner turns off. -_BLOCKING = ("id", "display_name") -_BLOCKING_SSH = ("host", "port", "user", "key") - -# Wanted, and reported, but never a refusal. ``software`` is the deciding one for -# picking a machine -- measured 2026-08-17, CalculiX runs only on the box with -# the A800s because the binary needs a glibc the 32-core box does not have, so a -# rule like "a CPU-only solver belongs on the CPU box" picks the one machine that -# cannot run it. ``budget_unit`` and ``concurrency`` belong to the machine and -# were being restated in every campaign's meta instead. -_WANTED = ("software", "budget_unit", "concurrency") - -_ID = re.compile(r"^[a-z0-9][a-z0-9_-]*$") - -# Where a machine's budget is metered: a property of the machine, so it is not -# restated in the meta of every campaign run on it. -_BUDGET_UNITS = ("minute", "core-minute", "gpu-minute") - -# Names that mean a field this file reads, spelled the way someone writing the -# file by hand reaches for first: without them a row written with ``name`` -# lists as its bare id, and nothing says why. -_MISSPELLED = { - "name": "display_name", - "hostname": "host", - "address": "host", - "username": "user", - "identity_file": "key", - "ssh_key": "key", - "gpu": "device", - "cpus": "cores", - "ram": "memory", -} - - -def transport_of(row: dict[str, Any]) -> str: - """``ssh`` or ``local``. Absent means ssh, which is what the runner assumes.""" - return LOCAL if str(row.get("transport") or SSH).strip().lower() == LOCAL else SSH - - -# How many devices a ``device`` line written as "2 x NVIDIA A800..." names. -_DEVICE_COUNT = re.compile(r"^\s*(\d+)\s*[xX]\s") -# A memory line such as "463 GB" or "1.5 TiB", read only when admission asks. -_MEMORY = re.compile(r"^\s*(\d+(?:\.\d+)?)\s*(TiB|TB|T|GiB|GB|G)\b", re.IGNORECASE) - - -def capacity(row: dict[str, Any]) -> dict[str, int]: - """What a row can hand out, in whole units. Empty when it says nothing countable. - - ``gpus`` is read first; absent, a ``device`` written as "N x ..." names the - count. ``cores`` is the row's own field. ``memory_gb`` is parsed from the - memory text when it parses at all. A row that hands out nothing here is - admitted by job count, the way every row was before capacity existed. - """ - out: dict[str, int] = {} - g = row.get("gpus") - if isinstance(g, int) and not isinstance(g, bool) and g >= 1: - out["gpus"] = g - elif str(row.get("kind") or "").strip().lower() == "gpu": - # The "N x ..." reading is for a GPU row only, as the admission design - # says: a CPU box whose device line reads "2 x Intel Xeon" hands out - # cores, and reading it as two devices would gate it on GPUs it has not. - m = _DEVICE_COUNT.match(str(row.get("device") or "")) - if m and int(m.group(1)) >= 1: - out["gpus"] = int(m.group(1)) - c = row.get("cores") - if isinstance(c, int) and not isinstance(c, bool) and c >= 1: - out["cores"] = c - m = _MEMORY.match(str(row.get("memory") or "")) - if m: - n = float(m.group(1)) - if m.group(2).lower().startswith("t"): - n *= 1024 - if n >= 1: - out["memory_gb"] = int(n) - return out - - -def resource_unit(row: dict[str, Any]) -> str: - """``gpus``, ``cores``, or "" for a row admitted by job count. - - A GPU machine's cores are not what its jobs contend for, so a row that hands - out devices is gated on those alone -- and a GPU row whose device count is - unknown is gated by job count, never by its cores (reviewed 2026-09-07: a - probed two-card box with no count was admitted by its 128 cores, sixteen - jobs deep). The doctor names such a row; `gpus: N` or a "N x ..." device - line moves it onto device admission. - """ - cap = capacity(row) - if "gpus" in cap: - return "gpus" - if str(row.get("kind") or "").strip().lower() == "gpu": - return "" - if "cores" in cap: - return "cores" - return "" - - -@dataclass(frozen=True) -class Problem: - """One thing wrong with a row. ``blocking`` means the machine cannot be used. - - The two levels are not decoration. A blocking problem is the ground a caller - refuses on -- an agent reading this list to pick a machine has nowhere to put - the work without one -- while the rest is worth telling the owner and worth - nobody's refusal. Folding them together would mean a registry that predates a field - reads as broken, and a check that condemns working machines gets turned off. - """ - - text: str - blocking: bool = False - - def __str__(self) -> str: - return self.text - - -def row_problems(row: dict[str, Any]) -> list[Problem]: - """Everything wrong with one row, in the owner's terms. Empty when it is fine. - - Judged here, next to the reader, rather than in whatever wrote the file. Every - connections.json that has existed so far was written by hand, and a - hand-written file has to be judged by what reads it or by nothing at all. - """ - rid = str(row.get("id") or "").strip() - label = rid or "" - out: list[Problem] = [] - raw_id = str(row.get("id") or "") - if not rid: - out.append(Problem("a machine here has no id", blocking=True)) - elif raw_id != rid: - # Consumers normalise differently: `shown` hands the raw id on, while - # whoever asks for a machine by name has usually stripped it, and - # `usable` collects its duplicate set on stripped ids, so two spellings - # of one id are seen as one. A padded id would otherwise read as usable - # and then be unselectable. Refused at the row instead of taught to - # every reader. - out.append(Problem(f"{label}: id has leading or trailing whitespace", blocking=True)) - elif not _ID.match(rid): - out.append(Problem(f"{label}: id must be lowercase letters, digits, '-' or '_'", blocking=True)) - for wrong, right in _MISSPELLED.items(): - if wrong in row and right not in row: - out.append(Problem(f"{label}: '{wrong}' is not a field this reads; it is spelled '{right}'")) - kind = transport_of(row) - blocking = _BLOCKING + (_BLOCKING_SSH if kind == SSH else ()) - for field in blocking: - if field != "id" and row.get(field) in (None, ""): - out.append(Problem(f"{label}: '{field}' is missing, so there is no way onto it", blocking=True)) - for field in _WANTED: - if field == "concurrency" and resource_unit(row): - continue - if row.get(field) in (None, ""): - out.append(Problem(f"{label}: '{field}' is not set")) - port = row.get("port") - if port not in (None, "") and (isinstance(port, bool) or not isinstance(port, int)): - out.append(Problem(f"{label}: 'port' must be a number, not {port!r}", blocking=True)) - elif isinstance(port, int) and not 1 <= port <= 65535: - out.append(Problem(f"{label}: 'port' {port} is not a port number", blocking=True)) - conc = row.get("concurrency") - if conc not in (None, "") and (isinstance(conc, bool) or not isinstance(conc, int) or conc < 1): - out.append(Problem(f"{label}: 'concurrency' must be a whole number of jobs, not {conc!r}")) - gp = row.get("gpus") - if gp not in (None, "") and (isinstance(gp, bool) or not isinstance(gp, int) or gp < 1): - out.append(Problem(f"{label}: 'gpus' must be a whole number of devices, not {gp!r}")) - held_unit = resource_unit(row) - if str(row.get("kind") or "").strip().lower() == "gpu" and held_unit != "gpus": - out.append( - Problem( - f"{label}: kind is gpu but neither 'gpus' nor a device written as 'N x ...' says how many " - "devices it hands out; jobs are admitted by job count until it does" - ) - ) - elif held_unit and conc not in (None, ""): - out.append( - Problem( - f"{label}: 'concurrency' is not read on a row that says {held_unit}; capacity decides how many jobs fit" - ) - ) - unit = row.get("budget_unit") - if unit not in (None, "") and str(unit) not in _BUDGET_UNITS: - out.append(Problem(f"{label}: 'budget_unit' should be one of {', '.join(_BUDGET_UNITS)}, not {unit!r}")) - paths = row.get("paths") - if paths is not None: - if not isinstance(paths, list): - out.append(Problem(f"{label}: 'paths' must be a list of absolute paths")) - else: - for claim in paths: - if not str(claim).startswith("/"): - out.append(Problem(f"{label}: path {claim!r} is not absolute")) - elif not _is_specific_enough(Path(str(claim))): - out.append(Problem(f"{label}: path {claim!r} names a filesystem rather than a place")) - return out - - -def usable(rows: list[dict[str, Any]]) -> list[dict[str, Any]]: - """The machines that can actually be run on. What a caller refuses over. - - A duplicated id takes every row that carries it out, not just the later - one: the id is what a campaign stores and what anything asking for a - machine by name looks up, so two rows behind one id make the reference - ambiguous -- which row answers depends on file order, and "the first one" - is an accident, not an answer. Judged here rather than only in - :func:`problems` because this is the list the tools that pick a machine and - the doctor's exit code key on; a blocking - diagnostic that leaves the registry usable is a warning nobody refuses - over (measured: two valid rows sharing an id kept usable=2 and doctor - exit 0 while problems() reported a blocking defect). - """ - ids = [str(r.get("id") or "").strip() for r in rows] - duplicated = {i for i in ids if i and ids.count(i) > 1} - return [ - r - for r in rows - if str(r.get("id") or "").strip() not in duplicated and not any(p.blocking for p in row_problems(r)) - ] - +# The reader, whole. Re-exported by name so every caller in this plugin keeps +# its ``connections.`` spelling. +from raven.ops.connection_add import ASK_OWNER +from raven.ops.connections import ( # noqa: F401 -- re-exports + LOCAL, + MISSING, + OK, + SHOWN, + SSH, + STORE, + UNREADABLE, + Problem, + Read, + capacity, + load, + problems, + read, + resource_unit, + row_problems, + shown, + store_path, + transport_of, + usable, +) -# What the agent may see. The key path is deliberately not in it: the agent never -# needs to authenticate, and a field it cannot use is one more thing to reason -# about wrongly. -# -# ``software`` is the deciding one and was not obvious. Measured 2026-08-17 on -# these two machines: CalculiX runs only on the box with the A800s, because the -# binary needs a glibc the 32-core box does not have. A rule like "a CPU-only -# solver belongs on the CPU box" would therefore pick the one machine that -# cannot run it. What a machine has installed decides; what it is made of only -# narrows. -_SHOWN = ("kind", "device", "gpus", "cores", "memory", "software", "budget_unit", "concurrency", "note") +_SHOWN = SHOWN # What the backend needs and the agent does not. ``transport`` is in here rather -# than in _SHOWN on purpose: whether a machine is reached over ssh or is simply +# than in SHOWN on purpose: whether a machine is reached over ssh or is simply # this one changes nothing about which machine the work calls for, and a field # the agent can see is a field it can reason about wrongly -- here, by deciding # it may skip naming the machine at all, which loses the record of where a @@ -292,101 +77,6 @@ def usable(rows: list[dict[str, Any]]) -> list[dict[str, Any]]: _TRANSPORT = ("host", "port", "user", "key", "transport") -# Points this instance at a registry that is not beside its own config. Set by -# a launcher that hosts raven inside another raven: the machines belong to the -# owner, not to whichever sub-agent is asking, and every copy taken to keep an -# instance supplied is a copy that stops being true the day the owner adds a -# machine. Five byte-identical copies existed on this computer on 2026-08-25, -# and the launcher that made them copied once and never again. -CONNECTIONS_ENV = "RAVEN_CONNECTIONS" - - -def store_path() -> Path: - """Where this instance reads its machines: the env var, else the owner's home. - - The trunk module resolves the fallback beside its own config file; a plugin - cannot read the host's config path, so the fallback is the default config - home the launcher points RAVEN_CONNECTIONS away from anyway (verdict C2: - trunk behaviour is the baseline, and the launcher always sets the env var - in a hosted install, so the fallback only serves a bare local run). - """ - override = os.environ.get(CONNECTIONS_ENV, "").strip() - if override: - return Path(override).expanduser() - return Path.home() / ".raven" / STORE - - -MISSING, UNREADABLE, OK = "missing", "unreadable", "ok" - - -@dataclass(frozen=True) -class Read: - """The registry as this instance found it, and why it looks that way. - - The state is separate from the rows because "there are no machines" and "the - machine list could not be read" are different facts and were being reported - as one. A hand-edited file with a stray comma parsed as ``ValueError``, - became ``[]``, and ``describe`` then told the loop that the owner had set no - machine up -- so a typo silently took the whole on-call surface out, and the - one thing the loop was told about it was false. - """ - - rows: list[dict[str, Any]] - state: str - detail: str = "" - - -def read() -> Read: - """The registry, with the reason behind an empty one. Never raises.""" - path = store_path() - try: - text = path.read_text(encoding="utf-8") - except FileNotFoundError: - return Read([], MISSING) - except OSError as exc: - return Read([], UNREADABLE, f"{path} could not be opened: {exc}") - try: - data = json.loads(text) - except ValueError as exc: - return Read([], UNREADABLE, f"{path} is not valid JSON: {exc}") - rows = data.get("connections") if isinstance(data, dict) else data - if not isinstance(rows, list): - return Read( - [], - UNREADABLE, - f"{path} should hold a list of machines, either as the whole file or " - "under a 'connections' key; it holds neither.", - ) - out = [r for r in rows if isinstance(r, dict) and str(r.get("id") or "").strip()] - dropped = len(rows) - len(out) - detail = f"{dropped} entr{'y' if dropped == 1 else 'ies'} in {path} had no id and were skipped." - return Read(out, OK, detail if dropped else "") - - -def load() -> list[dict[str, Any]]: - """Every connection, in file order. An unreadable or absent file is none.""" - return read().rows - - -def problems() -> list[Problem]: - """Everything wrong with the registry as a whole: the file, then each row.""" - found = read() - if found.state == UNREADABLE: - return [Problem(found.detail, blocking=True)] - out = [Problem(found.detail)] if found.detail else [] - seen: dict[str, int] = {} - for row in load(): - out.extend(row_problems(row)) - rid = str(row.get("id") or "").strip() - seen[rid] = seen.get(rid, 0) + 1 - out.extend( - Problem(f"{rid}: appears {n} times; an id has to name one machine", blocking=True) - for rid, n in seen.items() - if n > 1 - ) - return out - - def get(conn_id: str) -> dict[str, Any] | None: """One connection by its id, or None. The id is internal and never renamed. @@ -430,44 +120,13 @@ def resolve_into(meta: dict[str, Any]) -> dict[str, Any]: return merged -# What the owner has to be asked for, when there is nobody to ask it of here. -# Kept to what only they know: the address and the key are deliberately outside -# what the loop can see, and a name is theirs to give. Everything a machine can -# say about itself -- cores, memory, devices -- is read off it once reached, and -# ssh's own config resolves a port, a user or a key the owner left out. Once -# they have answered, the write is the loop's own: ops_connection_add. -_REQUEST = """\ -Ask the owner, in one message, and stop there until they answer (there is no -campaign yet, so end the turn with the question; whoever called you relays the -answer): - - is the machine this very computer, or another one reached over ssh? - - what do they call it? - - if another one: its address -- and port, username and private-key path - only if they know them; left out, ssh's own config is tried and whatever - connects is kept -Optional, only if they care to say: what is installed on it (with paths), -which directories on it hold their work, and how its budget is counted. - -With the answers, call ops_connection_add: it reaches the machine before -writing anything, and what the machine says about itself is read off it. -Until a machine is listed there is nothing to run on. Do not submit anything, -and do not look for a way in with exec or ssh.""" - - -def shown(row: dict[str, Any]) -> dict[str, Any]: - """One machine as anything outside may see it: no address, no credential. - - The same projection ``describe`` renders, handed over as data so a caller in - another process can put it in front of whoever needs it. The split is the - point of the function: what a machine *is* travels, and the way onto it does - not -- a field the reader cannot use is one more thing to reason about - wrongly, and here the reader may be a node writing a training script. - """ - out = {"id": str(row.get("id") or ""), "display_name": str(row.get("display_name") or row.get("id") or "")} - out.update({k: row[k] for k in _SHOWN if row.get(k) not in (None, "")}) - if isinstance(row.get("paths"), list): - out["paths"] = [str(x) for x in row["paths"]] - return out +# The ask is trunk's (the coding agent's guide points at the same text); what +# follows it is this product's: there is no campaign yet, so the turn ends on +# the question. +_REQUEST = ( + ASK_OWNER + "\nThere is no campaign yet, so end the turn with the question; whoever called you " + "relays the answer. Do not submit anything until a machine is listed." +) def describe() -> str: @@ -481,8 +140,9 @@ def describe() -> str: f"The machine registry for this instance cannot be read.\n{found.detail}\n\n" "This is NOT the same as having no machines: the file may well list " "several, and this instance cannot see any of them. Say exactly this " - "to the owner -- the file needs fixing by hand. Do not submit anything " - "and do not look for a way in yourself." + "to the owner -- the file needs fixing, or rewriting with " + "`raven ops connection add`. Do not submit anything and do not look " + "for a way in yourself." ) rows = load() if not rows: @@ -496,7 +156,7 @@ def describe() -> str: lines = [] for row in rows: bits = [f"{row.get('display_name') or row['id']} (id {row['id']})"] - bits += [f"{k} {row[k]}" for k in _SHOWN if row.get(k) not in (None, "")] + bits += [f"{k} {row[k]}" for k in SHOWN if row.get(k) not in (None, "")] lines.append(" " + " ".join(bits)) trouble = ( "\n\nOne or more of these cannot be used as written, and picking it will " @@ -551,208 +211,3 @@ def describe() -> str: "echo GPU=$(nvidia-smi --query-gpu=name,memory.total --format=csv,noheader 2>/dev/null | paste -sd'|' -); " "echo LIBC=$(ldd --version 2>/dev/null | head -1)" ) - - -def ssh_defaults(host: str, *, port: int = 0, user: str = "") -> dict[str, Any]: - """What ssh itself would use for this host: ``port``, ``user``, ``keys``. - - ``ssh -G`` prints the resolved configuration -- ``~/.ssh/config`` blocks - included -- so an owner who left the port, the user or the key path out - gets the values their own ssh would pick, not a guess. ``keys`` lists only - identity files that exist here, in ssh's own order; the caller probes each - and keeps the one that connects. Empty when ssh is not on this computer. - """ - import subprocess - - argv = ["ssh", "-G"] + (["-p", str(port)] if port else []) + [f"{user}@{host}" if user else host] - try: - done = subprocess.run(argv, capture_output=True, text=True, timeout=10, check=False) - except (FileNotFoundError, subprocess.TimeoutExpired): - return {} - if done.returncode != 0: - return {} - out: dict[str, Any] = {"keys": []} - for line in done.stdout.splitlines(): - key, _, value = line.partition(" ") - if key == "port" and value.strip().isdigit(): - out["port"] = int(value) - elif key == "user" and value.strip(): - out["user"] = value.strip() - elif key == "identityfile" and value.strip(): - if Path(os.path.expanduser(value.strip())).exists(): - out["keys"].append(value.strip()) - return out - - -def probe(row: dict[str, Any], *, timeout: float = 30.0, isolate_key: bool = False) -> tuple[bool, str, dict[str, Any]]: - """Reach the machine and read what it is. ``(reached, message, detected)``. - - The reaching is the point and the reading is the bonus. A row that cannot be - reached is not written, because an unreachable row in the registry is worse - than an absent one: absent is a question the loop knows to ask, unreachable - is a fact it acts on. Rides the plugin's own transport seam, so the way a - machine is reached here is the way its jobs will be reached. - - ``isolate_key`` is for the one caller that has to say WHICH key opened the - session rather than merely that one opened: trying the keys ssh named when - the owner gave no path. Without it, ``-i`` is a preference and not a - restriction -- the other configured identities and the agent are offered too - -- so the first candidate could be credited with a session a different key - authenticated, and the registry would then hold a path that stops working - the day that agent or config changes. - """ - from oncall_flow.backend import JobBackendError - from oncall_flow.transport import runner_from - - where = "this computer" if transport_of(row) == LOCAL else f"{row.get('user')}@{row.get('host')}:{row.get('port')}" - try: - if isolate_key and transport_of(row) == SSH: - from oncall_flow.docker_backend import make_ssh_runner - - runner = make_ssh_runner( - str(row.get("host") or ""), - int(row.get("port") or 22), - os.path.expanduser(str(row.get("key") or "")), - user=str(row.get("user") or "root"), - identities_only=True, - ) - else: - runner = runner_from(row, what="connection", cap_seconds=timeout) - code, out = runner(_PROBE) - except JobBackendError as exc: - return False, f"could not reach {where}: {exc}", {} - if code != 0: - return False, f"could not reach {where}: {out.strip() or f'exit {code}'}", {} - return True, f"reached {where}", _parse_probe(out) - - -def _parse_probe(out: str) -> dict[str, Any]: - seen: dict[str, str] = {} - for line in out.splitlines(): - key, _, value = line.partition("=") - if value.strip(): - seen[key.strip()] = value.strip() - found: dict[str, Any] = {} - if seen.get("CORES", "").isdigit(): - found["cores"] = int(seen["CORES"]) - if seen.get("MEM", "").isdigit() and int(seen["MEM"]) > 0: - found["memory"] = f"{seen['MEM']} GB" - gpu = seen.get("GPU", "") - if gpu: - cards = [c.strip() for c in gpu.split("|") if c.strip()] - found["gpus"] = len(cards) - found["device"] = f"{len(cards)} x {cards[0]}" if len(set(cards)) == 1 else " + ".join(cards) - found["kind"] = "gpu" - elif "cores" in found: - found["kind"] = "cpu" - if seen.get("LIBC"): - found["note"] = seen["LIBC"] - return found - - -def write(row: dict[str, Any]) -> Path: - """Append one machine, keeping whatever shape the file already had. - - Refuses rather than rewrites when the file holds entries this reader skips: - serialising the survivors would erase rows the owner wrote by hand, and a - recoverable typo is theirs to fix, not ours to delete on the way past. - """ - path = store_path() - found = read() - if found.state == UNREADABLE: - raise ValueError(f"{found.detail}\nFix or move that file before adding to it.") - if found.detail: - raise ValueError( - f"{found.detail}\nAdding a machine here would rewrite the file without them. " - "Give those entries an id (or remove them) first, then add this machine." - ) - rows = list(found.rows) - if any(str(r.get("id")).strip() == row["id"] for r in rows): - raise ValueError(f"a machine with id {row['id']!r} is already listed in {path}") - rows.append(row) - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps({"connections": rows}, indent=2, ensure_ascii=False) + "\n", encoding="utf-8") - return path - - -_ALIAS_MARK = "# raven connection {conn_id} (managed; rewritten on every add)" - - -def write_ssh_alias(row: dict[str, Any]) -> str | None: - """Keep a ``Host `` alias in ``~/.ssh/config`` for an ssh row. - - Transfers cannot ride the exec machine channel -- rsync and scp run their - client HERE and only address the machine -- so without an alias every - transfer carries the raw address into the model's context. One managed - block per id, replaced in full on re-add; everything outside the markers - is the owner's and is never touched. Returns the alias, or None for a - local row. A failure is the caller's to report as a warning, never as a - refusal: the alias is a convenience beside the registry, not part of it. - """ - if transport_of(row) == LOCAL: - return None - conn_id = str(row["id"]) - mark = _ALIAS_MARK.format(conn_id=conn_id) - block = "\n".join( - [ - mark, - f"Host {conn_id}", - f" HostName {row.get('host')}", - f" Port {int(row.get('port') or 22)}", - f" User {row.get('user') or 'root'}", - f" IdentityFile {row.get('key') or '~/.ssh/id_rsa'}", - mark, - ] - ) - path = Path(os.path.expanduser("~/.ssh/config")) - path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) - text = path.read_text(encoding="utf-8") if path.exists() else "" - if mark in text: - head, _, rest = text.partition(mark) - _, _, tail = rest.partition(mark) - text = head.rstrip("\n") + ("\n" if head.strip() else "") + tail.lstrip("\n") - text = (text.rstrip("\n") + "\n\n" if text.strip() else "") + block + "\n" - path.write_text(text, encoding="utf-8") - path.chmod(0o600) - return conn_id - - -# A claim shallower than this is a whole filesystem, not a case: "/", "/opt", -# "/Users/admin". Honouring one would put a line about machines on every ordinary -# look, and a note that fires everywhere is read as noise and then not read at -# all. Three components is the shallowest thing worth claiming -# ("/Evermind/bj_share/lxt" is four; "/srv/arena" is two and is allowed by the -# root list below rather than by depth). -_MIN_CLAIM_DEPTH = 2 -_NEVER_CLAIMED = frozenset( - { - "/", - "/usr", - "/opt", - "/etc", - "/var", - "/tmp", - "/bin", - "/sbin", - "/lib", - "/home", - "/Users", - "/Applications", - "/System", - "/Library", - "/private", - } -) - - -def _is_specific_enough(root: Path) -> bool: - """Whether a claimed root names a place rather than a filesystem.""" - text = str(root).rstrip("/") or "/" - if text in _NEVER_CLAIMED: - return False - parts = [p for p in root.parts if p not in ("/", "")] - if len(parts) < _MIN_CLAIM_DEPTH: - return False - # A home directory itself: /Users/admin, /home/me. Two components, and every - # ordinary look happens under it. - return not (len(parts) == 2 and f"/{parts[0]}" in ("/Users", "/home")) diff --git a/agents/raven-oncall/plugins/oncall-flow/oncall_flow/docker_backend.py b/agents/raven-oncall/plugins/oncall-flow/oncall_flow/docker_backend.py index 0f7351768..3697e1eec 100644 --- a/agents/raven-oncall/plugins/oncall-flow/oncall_flow/docker_backend.py +++ b/agents/raven-oncall/plugins/oncall-flow/oncall_flow/docker_backend.py @@ -227,13 +227,6 @@ def _parse_docker_time(value: str) -> float | None: return None -#: What it takes for a session to be attributable to the key passed with ``-i``: -#: no config read (so the owner's own IdentityFile lines are out), no agent, and -#: no provider identities. Read by the guard that measures this through ssh's own -#: resolver rather than by eye -- see the connection-add suite. -ISOLATE_IDENTITY = ("-F", "/dev/null", "-o", "IdentitiesOnly=yes", "-o", "IdentityAgent=none") - - def make_ssh_runner( host: str, port: int, @@ -241,21 +234,11 @@ def make_ssh_runner( *, user: str = "root", connect_timeout: int = 15, - identities_only: bool = False, ) -> CommandRunner: - """Run a command over ssh. ``identities_only`` narrows auth to ``key`` alone. - - Off by default, which is how a job reaches its machine: whatever the owner's - ssh would use gets to work, an agent included. Turned on only where the - session has to prove WHICH key opened it, and it takes all three options in - ISOLATE_IDENTITY to get there. ``-i`` is a preference, not a restriction; - ``IdentitiesOnly=yes`` is defined as excluding what an agent or provider adds, - not what the config names, so the owner's own ``IdentityFile`` lines are still - offered beside the candidate. Measured with OpenSSH 9.9p2 against a config - carrying two ``IdentityFile`` lines: ``-i cand`` with IdentitiesOnly and no - agent resolves to three identities, and only reading no config at all leaves - the one. An existing ``-i`` file suppresses the built-in defaults by itself. - """ + """Run a command over ssh, the way a job reaches its machine: whatever the + owner's ssh would use gets to work, an agent included. The probe that has + to prove WHICH key opened a session is trunk's + (``raven.ops.transport.make_ssh_runner(identities_only=True)``).""" import subprocess def run(cmd: str) -> tuple[int, str]: @@ -272,8 +255,6 @@ def run(cmd: str) -> tuple[int, str]: "-o", "StrictHostKeyChecking=accept-new", ] - if identities_only: - argv += list(ISOLATE_IDENTITY) argv += [ f"{user}@{host}", cmd, diff --git a/agents/raven-oncall/plugins/oncall-flow/oncall_flow/tools/__init__.py b/agents/raven-oncall/plugins/oncall-flow/oncall_flow/tools/__init__.py index e8c305a94..52ba72c54 100644 --- a/agents/raven-oncall/plugins/oncall-flow/oncall_flow/tools/__init__.py +++ b/agents/raven-oncall/plugins/oncall-flow/oncall_flow/tools/__init__.py @@ -102,14 +102,6 @@ def make_ops_connections(ctx: "PluginContext"): return base.adopt(OpsConnectionsTool()) -def make_ops_connection_add(ctx: "PluginContext"): - if not _wired(ctx): - return None - from oncall_flow.tools.ops_connection_add import OpsConnectionAddTool - - return base.adopt(OpsConnectionAddTool()) - - def make_ops_declare(ctx: "PluginContext"): if not _wired(ctx): return None diff --git a/agents/raven-oncall/plugins/oncall-flow/raven-plugin.toml b/agents/raven-oncall/plugins/oncall-flow/raven-plugin.toml index 31f05731e..f20e3da60 100644 --- a/agents/raven-oncall/plugins/oncall-flow/raven-plugin.toml +++ b/agents/raven-oncall/plugins/oncall-flow/raven-plugin.toml @@ -49,7 +49,8 @@ factory = "oncall_flow.flow:make_flow_hook" # is trunk exec's own `machine` parameter (the same-name-shadow ruling closed # by moving the face into the trunk tool), and ops_tune_launch stays off the # menu on purpose (full-auto never competes with the round-by-round loop; -# `python -m oncall_flow.tune` is the D5 entry). +# `python -m oncall_flow.tune` is the D5 entry). ops_connection_add is trunk's +# tool since 2026-09-23, served on tools.connectionAdd in this product's config. [[plugin.contributes.tools]] name = "ops_tune_status" @@ -75,10 +76,6 @@ factory = "oncall_flow.tools:make_ops_campaigns" name = "ops_connections" factory = "oncall_flow.tools:make_ops_connections" -[[plugin.contributes.tools]] -name = "ops_connection_add" -factory = "oncall_flow.tools:make_ops_connection_add" - [[plugin.contributes.tools]] name = "ops_declare" factory = "oncall_flow.tools:make_ops_declare" diff --git a/docs-site/docs/oncall.md b/docs-site/docs/oncall.md index fbc80ddab..8545aa5c7 100644 --- a/docs-site/docs/oncall.md +++ b/docs-site/docs/oncall.md @@ -50,10 +50,21 @@ raven ops connection add --id local-lab --name "Local lab" \ --non-interactive ``` -Registration writes `connections.json` beside the active config, or the path -selected by `RAVEN_CONNECTIONS`. SSH registration can also write a managed -alias in `~/.ssh/config`; the key path is recorded, not the private key bytes. -Do not put keys or passwords in a task message. +Registration writes `connections.json` in the raven home (`RAVEN_HOME`, or +`~/.raven`), or the path selected by `RAVEN_CONNECTIONS`. A registry beside a +config kept elsewhere is still read when the home has none. Sub-agents run on a +rendered config in a state directory of their own and inherit the home, so +they read the owner's registry rather than a copy of it. SSH registration can +also write a managed alias in `~/.ssh/config`; the key path is recorded, not +the private key bytes. Do not put keys or passwords in a task message. + +An agent can register a machine too. The `ops_connection_add` tool (on in +raven-oncall and raven-code through `tools.connectionAdd`; off elsewhere) takes +what the owner said in conversation -- this computer or another, what they +call it, an address if another -- fills a port, user or key they left out from +their own ssh config, reaches the machine, and only then writes the row. When +the registry lists no machine that fits, the agent asks the owner rather than +looking for a way in with a raw ssh command. `doctor` validates registry usability; it is not a fresh execution test of every remote environment. `--skip-probe` on `add` skips contacting the machine @@ -157,4 +168,5 @@ A “done” message without evidence is not a benchmark result. | Restart left uncertain state | Read ledger and remote status before retrying or killing anything | Implementation evidence lives in `agents/raven-oncall/plugins/oncall-flow/`, -`raven/ops/connections.py`, and `raven/cli/ops_connection_commands.py`. +`raven/ops/connections.py`, `raven/ops/connection_add.py`, +`raven/agent/tools/connection_add.py`, and `raven/cli/ops_connection_commands.py`. diff --git a/docs-site/docs/oncall.zh.md b/docs-site/docs/oncall.zh.md index 8381c4891..dd5d42653 100644 --- a/docs-site/docs/oncall.zh.md +++ b/docs-site/docs/oncall.zh.md @@ -41,9 +41,17 @@ raven ops connection add --id local-lab --name "Local lab" \ --non-interactive ``` -注册会写入当前 config 旁的 `connections.json`，或 `RAVEN_CONNECTIONS` 指定位置。 -SSH 注册还可能在 `~/.ssh/config` 写入受管理的别名；只记录私钥路径，不记录私钥内容。 -不要把密钥或密码放进任务消息。 +注册会写入 raven 家目录（`RAVEN_HOME`，默认 `~/.raven`）下的 `connections.json`， +或 `RAVEN_CONNECTIONS` 指定位置；家目录里没有时，仍会读放在别处 config 旁的那份。 +子代理各自用渲染出的 config 跑在自己的状态目录里、继承家目录，所以读到的是 +主人的注册表，不是一份副本。SSH 注册还可能在 `~/.ssh/config` 写入受管理的别名； +只记录私钥路径，不记录私钥内容。不要把密钥或密码放进任务消息。 + +agent 也能登记机器。`ops_connection_add` 工具（raven-oncall 与 raven-code 通过 +`tools.connectionAdd` 开启，其他产品关闭）接主人在对话里说的话——是这台电脑还是 +另一台、叫什么、另一台的话地址是什么——用主人自己的 ssh 配置补上没说的端口、 +用户名或密钥路径，先连上机器，连上了才写入。注册表里没有合适的机器时，agent +会问主人，而不是拿一条原始 ssh 命令自己找路进去。 `doctor` 校验注册表可用性，不会重新执行每台远程环境的连通测试。 `add --skip-probe` 不联系机器，并明确保留“未验证连接”状态。 @@ -135,4 +143,5 @@ Oncall 的声明、执行检查和升级策略，与宿主[工具权限](permiss | 重启后状态不确定 | 重试或停止前读取 ledger 与远程状态 | 实现依据位于 `agents/raven-oncall/plugins/oncall-flow/`、 -`raven/ops/connections.py` 和 `raven/cli/ops_connection_commands.py`。 +`raven/ops/connections.py`、`raven/ops/connection_add.py`、 +`raven/agent/tools/connection_add.py` 和 `raven/cli/ops_connection_commands.py`。 diff --git a/raven/agent/loop/_shared.py b/raven/agent/loop/_shared.py index 52508c09d..f1fed4050 100644 --- a/raven/agent/loop/_shared.py +++ b/raven/agent/loop/_shared.py @@ -42,6 +42,7 @@ from raven.agent.subagent.direct_chat import DirectChatHandoff from raven.agent.subagent.spawn_tool import SpawnTool from raven.agent.tools.ask_user import AskUserTool +from raven.agent.tools.connection_add import ConnectionAddTool from raven.agent.tools.deep_research import ( DeepResearchManager, DeepResearchOfferTool, diff --git a/raven/agent/loop/bundles.py b/raven/agent/loop/bundles.py index 8abf9549d..34361b1f8 100644 --- a/raven/agent/loop/bundles.py +++ b/raven/agent/loop/bundles.py @@ -25,6 +25,9 @@ class ToolWiring: web_fetch_provider: str = "jina" web_provider_keys: dict[str, str] | None = None image_search: bool = False + # Serve the registry-writing tool (``tools.connectionAdd``); off unless the + # product asks, see ``ToolsConfig.connection_add``. + connection_add: bool = False restrict_to_workspace: bool = False disabled_tools: list[str] | None = None tool_search_config: Any = None diff --git a/raven/agent/loop/main.py b/raven/agent/loop/main.py index 812916080..d2a8d0a7d 100644 --- a/raven/agent/loop/main.py +++ b/raven/agent/loop/main.py @@ -183,6 +183,7 @@ def __init__( web_fetch_provider = tools.web_fetch_provider web_provider_keys = tools.web_provider_keys image_search = tools.image_search + connection_add = tools.connection_add restrict_to_workspace = tools.restrict_to_workspace disabled_tools = tools.disabled_tools tool_search_config = tools.tool_search_config @@ -265,6 +266,7 @@ def __init__( self._boot_web_fetch_provider = web_fetch_provider self.web_provider_keys = web_provider_keys self.image_search = image_search + self.connection_add = connection_add from raven.config.raven import MemoryConfig, SubagentDagConfig, SubagentQuestionsConfig from raven.config.schema import DeepResearchToolConfig, MediaGenConfig diff --git a/raven/agent/loop/wiring.py b/raven/agent/loop/wiring.py index d0790d7b5..56183f431 100644 --- a/raven/agent/loop/wiring.py +++ b/raven/agent/loop/wiring.py @@ -14,6 +14,7 @@ Any, AskUserTool, Callable, + ConnectionAddTool, DeepResearchManager, EditFileTool, ExecTool, @@ -938,6 +939,13 @@ def _register_default_tools(self) -> None: extra_allowed_dirs=(self.workspace,), ) ) + # The registry writer beside exec's machine channel, for the products + # that run work on the owner's machines. Registered on the flag rather + # than withheld: this is a deploy-time product choice, not a credential + # that may arrive mid-session, and five product tool faces are pinned + # against a tool boarding uninvited. + if self.connection_add: + self.tools.register(ConnectionAddTool()) # web_search and the media tools register whatever the config holds and # are *withheld* while their section asks for nothing -- see # ``_unconfigured_tool_names``. Registration used to be the gate, which diff --git a/agents/raven-oncall/plugins/oncall-flow/oncall_flow/tools/ops_connection_add.py b/raven/agent/tools/connection_add.py similarity index 70% rename from agents/raven-oncall/plugins/oncall-flow/oncall_flow/tools/ops_connection_add.py rename to raven/agent/tools/connection_add.py index 14e562a08..f1ad3276f 100644 --- a/agents/raven-oncall/plugins/oncall-flow/oncall_flow/tools/ops_connection_add.py +++ b/raven/agent/tools/connection_add.py @@ -1,18 +1,17 @@ -"""Add one machine to the registry, from what the owner answered. - -The counterpart of ``ops_connections``' empty-registry reply: that reply asks -the owner what only they know, and this face takes the answer. Until 2026-09-06 -the host ran ``raven ops connection add --non-interactive`` on the owner's -behalf before dispatching a spawn; that step went with the pre-dispatch registry -gate, so an owner who answered in conversation had nothing consuming the answer. Here the write belongs to the instance that needs the machine. - -Two things keep the ask short. What a machine can say about itself -- cores, -memory, devices -- is read off it once reached, never typed. And what the owner -leaves out of the way in -- port, username, key path -- is resolved the way -their own terminal would resolve it (``ssh -G``, config blocks included), then -tried: the probe decides, and only what connected is written. Nothing is written -until the machine itself answers; an unreachable row in the registry is a fact -every later turn acts on, an absent one is a question the loop knows to ask. +"""Add one machine to the owner's registry, from what the owner answered. + +The counterpart of an empty listing: the listing asks the owner what only they +know, and this face takes the answer. Until 2026-09-06 the host ran ``raven ops +connection add --non-interactive`` on the owner's behalf before dispatching a +sub-agent; that step went with the pre-dispatch registry gate, and the write +moved into the on-call product. It is trunk's now because the coding agent +runs on the owner's machines too -- a case is built where the solver is, and +smoke-tested there -- and it runs BEFORE the on-call agent in the usual chain, +so a registry nobody has written yet has to be writable from where the need +first shows up. + +Opt-in per product (``tools.connectionAdd``): a lane that never runs work on +the owner's machines has no use for a tool that writes to their ssh config. """ from __future__ import annotations @@ -21,28 +20,31 @@ from typing import Any from raven.contracts.tool import Tool +from raven.ops.connections import _BUDGET_UNITS -_BUDGET_UNITS = ("minute", "core-minute", "gpu-minute") +TOOL_NAME = "ops_connection_add" -class OpsConnectionAddTool(Tool): +class ConnectionAddTool(Tool): """Write one machine the owner described, after reaching it.""" @property def name(self) -> str: - return "ops_connection_add" + return TOOL_NAME @property def description(self) -> str: return ( - "Add a machine to the registry from the owner's own answers, when ops_connections " - "lists none that fits. Needed: what they call it, and whether it is this very computer " - "or another one reached over ssh; for ssh, its address. Port, username and private-key " - "path only when the owner gave them -- left out, ssh's own config is consulted and " - "whatever connects is kept, so never invent one: a guess that lands in the registry " - "stops being a guess. What is installed, which directories hold their work and how the " - "budget is counted are optional; cores, memory and devices are read off the machine. " - "Nothing is written until the machine answers. Then hand the new id to ops_declare." + "Add a machine to the owner's registry from their own answers, when the machines " + "listed include none that fits -- or none is listed at all. Needed: what they call it, " + "and whether it is this very computer or another one reached over ssh; for ssh, its " + "address. Port, username and private-key path only when the owner gave them -- left " + "out, ssh's own config is consulted and whatever connects is kept, so never invent one: " + "a guess that lands in the registry stops being a guess. What is installed, which " + "directories hold their work and how the budget is counted are optional; cores, memory " + "and devices are read off the machine. Nothing is written until the machine answers. " + "Then name the machine by the new id wherever one is asked for: exec's 'machine' " + "parameter, or the on-call agent's ops_declare." ) @property @@ -104,7 +106,8 @@ async def execute( note: str = "", **kwargs: Any, ) -> str: - from oncall_flow import connections + from raven.ops import connection_add as adder + from raven.ops import connections from raven.utils.paths import mint_slug label = str(name).strip() @@ -114,7 +117,7 @@ async def execute( conn_id = str(id).strip() or mint_slug(label) row: dict[str, Any] = {"id": conn_id, "display_name": label, "transport": kind} if kind == connections.LOCAL: - reached, message, found = await asyncio.to_thread(connections.probe, row) + reached, message, found = await asyncio.to_thread(adder.probe, row) else: host = str(host).strip() if not host: @@ -126,7 +129,7 @@ async def execute( given = str(key).strip() resolved: dict[str, Any] = {} if not (port and str(user).strip() and given): - resolved = await asyncio.to_thread(connections.ssh_defaults, host, port=int(port or 0), user=str(user)) + resolved = await asyncio.to_thread(adder.ssh_defaults, host, port=int(port or 0), user=str(user)) row.update( { "host": host, @@ -143,20 +146,21 @@ async def execute( reached, message, found = False, "", {} for candidate in keys: row["key"] = candidate - # A path the owner named is their assertion, and is probed the way a - # job will reach the machine -- an agent doing the authenticating is - # their setup working. A path this tool picked is a claim of its own, - # so it is probed with that key alone: credited otherwise, the row - # would hold a key that only looked like the one that opened the - # session, and stop working the day the agent or the config changes. - reached, message, found = await asyncio.to_thread(connections.probe, row, isolate_key=not given) + # A path the owner named is their assertion, and is probed the way + # work will reach the machine -- an agent doing the authenticating + # is their setup working. A path this tool picked is a claim of its + # own, so it is probed with that key alone: credited otherwise, the + # row would hold a key that only looked like the one that opened + # the session, and stop working the day the agent or the config + # changes. + reached, message, found = await asyncio.to_thread(adder.probe, row, isolate_key=not given) if reached: break if not reached and not given: # Attribution costs the owner's ssh config: a candidate is tried # with no config read, so an alias or a jump host that makes this # machine reachable is not in play here. Naming the key puts the - # probe back on the path a job will take. + # probe back on the path work will take. message = ( f"{message} (tried, each on its own, the key(s) ssh named: {', '.join(keys)}; " "a candidate is probed with no ssh config read, so a machine reached through " @@ -182,7 +186,7 @@ async def execute( if blocking: return "REFUSED: " + "; ".join(str(f) for f in blocking) + ". Nothing was written." try: - written = connections.write(row) + written = adder.write(row) except ValueError as exc: return f"REFUSED: {exc} Nothing was written." @@ -193,11 +197,17 @@ async def execute( for fault in faults: lines.append(f"Worth telling the owner: {fault}") try: - alias = connections.write_ssh_alias(row) + alias = adder.write_ssh_alias(row) except OSError as exc: lines.append(f"ssh alias not written ({exc}); transfers will need the address by hand.") else: if alias: lines.append(f"ssh alias '{alias}' written to ~/.ssh/config, so rsync/scp reach it by id.") - lines.append(f"Now hand '{conn_id}' to ops_declare as 'connection'.") + lines.append( + f"Now name the machine as '{conn_id}': exec's 'machine' parameter runs a command there, " + "and the on-call agent's ops_declare takes it as 'connection'." + ) return "\n".join(lines) + + +__all__ = ["TOOL_NAME", "ConnectionAddTool"] diff --git a/raven/cli/ops_connection_commands.py b/raven/cli/ops_connection_commands.py index 36d181b20..f5787c964 100644 --- a/raven/cli/ops_connection_commands.py +++ b/raven/cli/ops_connection_commands.py @@ -26,132 +26,25 @@ from __future__ import annotations import json -import os import shlex -import subprocess -from pathlib import Path from typing import Any import typer from rich.console import Console from rich.table import Table +# One probe, one writer, one alias, whichever door knocks: the same functions +# serve the ``ops_connection_add`` tool. Bound here by name so this module's +# ``add`` reads them as its own globals -- which is also what a test that +# stands in a reachable machine patches. +from raven.ops.connection_add import parse_probe as _parse_probe # noqa: F401 -- re-export, the tests read it here +from raven.ops.connection_add import probe +from raven.ops.connection_add import write as _write +from raven.ops.connection_add import write_ssh_alias as _write_ssh_alias + connection_app = typer.Typer(help="The machines this instance can run work on.") console = Console() -_PROBE = ( - "echo CORES=$(nproc 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null); " - "echo MEM=$(free -g 2>/dev/null | awk '/^Mem:/{print $2}'); " - "echo GPU=$(nvidia-smi --query-gpu=name,memory.total --format=csv,noheader 2>/dev/null | paste -sd'|' -); " - "echo LIBC=$(ldd --version 2>/dev/null | head -1)" -) - - -def _run(argv: list[str], timeout: float) -> tuple[int, str, str]: - try: - done = subprocess.run(argv, capture_output=True, text=True, timeout=timeout, check=False) - except FileNotFoundError: - return 127, "", f"{argv[0]} not found on this computer" - except subprocess.TimeoutExpired: - return 124, "", f"timed out after {timeout:.0f}s" - return done.returncode, done.stdout, done.stderr - - -def _ssh_argv(host: str, port: int, user: str, key: str, command: str) -> list[str]: - return [ - "ssh", - "-o", - "BatchMode=yes", - "-o", - "ConnectTimeout=10", - "-o", - "StrictHostKeyChecking=accept-new", - "-p", - str(port), - "-i", - os.path.expanduser(key), - f"{user}@{host}", - command, - ] - - -def probe(row: dict[str, Any], *, timeout: float = 30.0) -> tuple[bool, str, dict[str, Any]]: - """Reach the machine and read what it is. ``(reached, message, detected)``. - - The reaching is the point and the reading is the bonus. A row that cannot be - reached is not written, because an unreachable row in the registry is worse - than an absent one: absent is a question the loop knows to ask, unreachable - is a fact it acts on. - """ - from raven.ops.connections import LOCAL, transport_of - - if transport_of(row) == LOCAL: - code, out, err = _run(["sh", "-c", _PROBE], timeout) - where = "this computer" - else: - argv = _ssh_argv(str(row["host"]), int(row["port"]), str(row["user"]), str(row["key"]), _PROBE) - code, out, err = _run(argv, timeout) - where = f"{row['user']}@{row['host']}:{row['port']}" - if code != 0: - return False, f"could not reach {where}: {(err or out).strip() or f'exit {code}'}", {} - return True, f"reached {where}", _parse_probe(out) - - -def _parse_probe(out: str) -> dict[str, Any]: - seen: dict[str, str] = {} - for line in out.splitlines(): - key, _, value = line.partition("=") - if value.strip(): - seen[key.strip()] = value.strip() - found: dict[str, Any] = {} - if seen.get("CORES", "").isdigit(): - found["cores"] = int(seen["CORES"]) - if seen.get("MEM", "").isdigit() and int(seen["MEM"]) > 0: - found["memory"] = f"{seen['MEM']} GB" - gpu = seen.get("GPU", "") - if gpu: - cards = [c.strip() for c in gpu.split("|") if c.strip()] - # The count is what admission reads (`gpus`), and the device line is - # written as "N x " when the cards match, which is the other - # spelling the readers infer a count from. Left as a "+"-joined list, - # the row a real two-card box probed to (2026-09-07) had no count at all, - # and admission fell back to job count on a machine it could have gated - # by device. - found["gpus"] = len(cards) - found["device"] = f"{len(cards)} x {cards[0]}" if len(set(cards)) == 1 else " + ".join(cards) - found["kind"] = "gpu" - elif "cores" in found: - found["kind"] = "cpu" - if seen.get("LIBC"): - found["note"] = seen["LIBC"] - return found - - -def _write(row: dict[str, Any]) -> Path: - """Append one machine, keeping whatever shape the file already had.""" - from raven.ops.connections import read, store_path - - path = store_path() - found = read() - if found.state == "unreadable": - raise ValueError(f"{found.detail}\nFix or move that file before adding to it.") - # `read` filters entries without an id and says so in `detail`; rewriting - # from its rows would serialize the survivors and erase the rest. The owner - # hand-wrote those, and a recoverable typo is theirs to fix, not ours to - # delete on the way past. - if found.detail: - raise ValueError( - f"{found.detail}\nAdding a machine here would rewrite the file without them. " - "Give those entries an id (or remove them) first, then add this machine." - ) - rows = list(found.rows) - if any(str(r.get("id")).strip() == row["id"] for r in rows): - raise ValueError(f"a machine with id {row['id']!r} is already listed in {path}") - rows.append(row) - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps({"connections": rows}, indent=2, ensure_ascii=False) + "\n", encoding="utf-8") - return path - def _slug(text: str) -> str: from raven.utils.paths import mint_slug @@ -159,56 +52,6 @@ def _slug(text: str) -> str: return mint_slug(text) -_ALIAS_MARK = "# raven connection {conn_id} (managed; rewritten on every add)" - - -def _write_ssh_alias(row: dict[str, Any]) -> str | None: - """Keep a ``Host `` alias in ``~/.ssh/config`` for an ssh row. - - Transfers cannot ride the exec machine channel -- rsync and scp run their - client HERE and only address the machine -- so without an alias every - transfer carries the raw address, and the raw address then has to live in - the model's context (measured 2026-09-03: a task statement shipped the - ssh string because nothing else could address the machine). With the - alias, ``rsync ... :...`` resolves inside ssh's own config and the - context carries only the registry name. - - One managed block per id, replaced in full on re-add; everything outside - the markers is the owner's and is never touched. Returns the alias, or - None when there is nothing to write (a local row) -- a failure is - reported by the caller as a warning, never as a refusal: the alias is a - convenience beside the registry, not part of it. - """ - from raven.ops.connections import LOCAL, transport_of - - if transport_of(row) == LOCAL: - return None - conn_id = str(row["id"]) - mark = _ALIAS_MARK.format(conn_id=conn_id) - block = "\n".join( - [ - mark, - f"Host {conn_id}", - f" HostName {row.get('host')}", - f" Port {int(row.get('port') or 22)}", - f" User {row.get('user') or 'root'}", - f" IdentityFile {row.get('key') or '~/.ssh/id_rsa'}", - mark, - ] - ) - path = Path(os.path.expanduser("~/.ssh/config")) - path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) - text = path.read_text(encoding="utf-8") if path.exists() else "" - if mark in text: - head, _, rest = text.partition(mark) - _, _, tail = rest.partition(mark) - text = head.rstrip("\n") + ("\n" if head.strip() else "") + tail.lstrip("\n") - text = (text.rstrip("\n") + "\n\n" if text.strip() else "") + block + "\n" - path.write_text(text, encoding="utf-8") - path.chmod(0o600) - return conn_id - - def _ask(label: str, default: str = "", *, required: bool = True) -> str: while True: got = str(typer.prompt(label, default=default, show_default=bool(default))).strip() diff --git a/raven/config/schema.py b/raven/config/schema.py index c168901c9..03850ed0e 100644 --- a/raven/config/schema.py +++ b/raven/config/schema.py @@ -1329,6 +1329,12 @@ class ToolsConfig(Base): mcp_servers: dict[str, MCPServerConfig] = Field(default_factory=dict) sandbox: SandboxConfig = Field(default_factory=SandboxConfig) tool_search: ToolSearchConfig = Field(default_factory=ToolSearchConfig) + connection_add: bool = False + """Serve ``ops_connection_add``, the tool that writes a machine the owner + describes into their connection registry after reaching it. Off by default: + a lane that never runs work on the owner's machines has no use for a tool + that writes to their ``~/.ssh/config``. The two products that build or run + work there (raven-code, raven-oncall) turn it on in their own config.""" disabled_tools: list[str] = Field(default_factory=list) """Tool names to withhold from the assembled tool array and refuse at dispatch. The general off switch for a tool this deploy does not want, and the only one diff --git a/raven/core/runtime.py b/raven/core/runtime.py index 6f2c58541..a35448a53 100644 --- a/raven/core/runtime.py +++ b/raven/core/runtime.py @@ -239,6 +239,7 @@ def build_runtime( web_fetch_provider=config.tools.web.fetch.provider, web_provider_keys=config.tools.web.vendor_keys(), image_search=config.tools.web.search.images, + connection_add=config.tools.connection_add, media_config=config.effective_media_config(), deep_research_config=config.tools.deep_research, exec_config=config.tools.exec, diff --git a/raven/ops/connection_add.py b/raven/ops/connection_add.py new file mode 100644 index 000000000..9bb6f2c43 --- /dev/null +++ b/raven/ops/connection_add.py @@ -0,0 +1,243 @@ +"""The write half of the machine registry: reach a machine, then write it down. + +:mod:`raven.ops.connections` reads the owner's machines and stays read-only on +purpose. This module is the one door that adds a row, and it is the same door +whichever surface knocks: the owner in a terminal (``raven ops connection +add``), or an agent that was told about a machine in conversation +(``ops_connection_add``). Until 2026-09-06 the host ran the command on the +owner's behalf before dispatching a sub-agent; that step went with the +pre-dispatch registry gate, and the write moved to the on-call product's own +plugin. It sits here now because the coding agent runs on the owner's machines +too -- it builds a case where the solver is and smoke-tests it there -- and a +second copy of the probe in a second plugin is the drift this tree already paid +for once (five byte-identical registries on one computer, 2026-08-25). + +Two things keep what the owner is asked short. What a machine can say about +itself -- cores, memory, devices -- is read off it once reached, never typed. +And what the owner leaves out of the way in -- port, username, key path -- is +resolved the way their own terminal would resolve it (``ssh -G``, config +blocks included), then tried: the probe decides, and only what connected is +written. Nothing is written until the machine itself answers; an unreachable +row in the registry is a fact every later turn acts on, an absent one is a +question the loop knows to ask. +""" + +from __future__ import annotations + +import json +import os +import subprocess +from pathlib import Path +from typing import Any + +# The reader is reached through its module at call time, not bound at import: +# where the store is gets decided per call (see ``connections.store_path``), +# and a test that stands a scratch registry in patches it there. +from raven.ops import connections +from raven.ops.connections import LOCAL, SSH, UNREADABLE, transport_of + +# What the owner has to be asked for, when the registry lists nothing that +# fits. Shared text: the on-call agent's listing reply and the coding agent's +# guide both point at it, so the questions are spelled once. The loop cannot +# fill any of it in -- the address and the key are deliberately outside what it +# can see -- and it must not guess: a guess that lands in the registry stops +# being a guess and becomes what every later turn reads as fact. +ASK_OWNER = """\ +Ask the owner, in one message, and stop there until they answer: + - is the machine this very computer, or another one reached over ssh? + - what do they call it? + - if another one: its address -- and port, username and private-key path + only if they know them; left out, ssh's own config is tried and whatever + connects is kept +Optional, only if they care to say: what is installed on it (with paths), +which directories on it hold their work, and how its budget is counted. + +With the answers, call ops_connection_add: it reaches the machine before +writing anything, and what the machine says about itself is read off it. +Until a machine is listed there is nothing to run on there. Do not look for a +way in yourself -- not with exec, not with an ssh typed from an address in the +task statement.""" + +# What the machine is asked about itself. Read once reached, never typed twice. +PROBE_SCRIPT = ( + "echo CORES=$(nproc 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null); " + "echo MEM=$(free -g 2>/dev/null | awk '/^Mem:/{print $2}'); " + "echo GPU=$(nvidia-smi --query-gpu=name,memory.total --format=csv,noheader 2>/dev/null | paste -sd'|' -); " + "echo LIBC=$(ldd --version 2>/dev/null | head -1)" +) + + +def ssh_defaults(host: str, *, port: int = 0, user: str = "") -> dict[str, Any]: + """What ssh itself would use for this host: ``port``, ``user``, ``keys``. + + ``ssh -G`` prints the resolved configuration -- ``~/.ssh/config`` blocks + included -- so an owner who left the port, the user or the key path out + gets the values their own ssh would pick, not a guess. ``keys`` lists only + identity files that exist here, in ssh's own order; the caller probes each + and keeps the one that connects. Empty when ssh is not on this computer. + """ + argv = ["ssh", "-G"] + (["-p", str(port)] if port else []) + [f"{user}@{host}" if user else host] + try: + done = subprocess.run(argv, capture_output=True, text=True, timeout=10, check=False) + except (FileNotFoundError, subprocess.TimeoutExpired): + return {} + if done.returncode != 0: + return {} + out: dict[str, Any] = {"keys": []} + for line in done.stdout.splitlines(): + key, _, value = line.partition(" ") + if key == "port" and value.strip().isdigit(): + out["port"] = int(value) + elif key == "user" and value.strip(): + out["user"] = value.strip() + elif key == "identityfile" and value.strip() and Path(os.path.expanduser(value.strip())).exists(): + out["keys"].append(value.strip()) + return out + + +def probe(row: dict[str, Any], *, timeout: float = 30.0, isolate_key: bool = False) -> tuple[bool, str, dict[str, Any]]: + """Reach the machine and read what it is. ``(reached, message, detected)``. + + The reaching is the point and the reading is the bonus. A row that cannot be + reached is not written, because an unreachable row in the registry is worse + than an absent one: absent is a question the loop knows to ask, unreachable + is a fact it acts on. Rides :mod:`raven.ops.transport`, the seam ``exec``'s + machine channel reaches a machine through, so the way a machine is reached + here is the way it will be reached afterwards. + + ``isolate_key`` is for the one caller that has to say WHICH key opened the + session rather than merely that one opened: trying the keys ssh named when + the owner gave no path. Without it ``-i`` is a preference and not a + restriction -- the other configured identities and the agent are offered too + -- so the first candidate could be credited with a session a different key + authenticated, and the registry would then hold a path that stops working + the day that agent or config changes. + """ + from raven.ops.transport import TransportError, make_ssh_runner, runner_from + + where = "this computer" if transport_of(row) == LOCAL else f"{row.get('user')}@{row.get('host')}:{row.get('port')}" + try: + if isolate_key and transport_of(row) == SSH: + runner = make_ssh_runner( + str(row.get("host") or ""), + int(row.get("port") or 22), + os.path.expanduser(str(row.get("key") or "")), + user=str(row.get("user") or "root"), + identities_only=True, + ) + else: + runner = runner_from(row, cap_seconds=timeout) + code, out = runner(PROBE_SCRIPT) + except TransportError as exc: + return False, f"could not reach {where}: {exc}", {} + except FileNotFoundError: + return False, f"could not reach {where}: ssh not found on this computer", {} + if code != 0: + return False, f"could not reach {where}: {out.strip() or f'exit {code}'}", {} + return True, f"reached {where}", parse_probe(out) + + +def parse_probe(out: str) -> dict[str, Any]: + """The row fields a probe's output names. Only what parsed cleanly.""" + seen: dict[str, str] = {} + for line in out.splitlines(): + key, _, value = line.partition("=") + if value.strip(): + seen[key.strip()] = value.strip() + found: dict[str, Any] = {} + if seen.get("CORES", "").isdigit(): + found["cores"] = int(seen["CORES"]) + if seen.get("MEM", "").isdigit() and int(seen["MEM"]) > 0: + found["memory"] = f"{seen['MEM']} GB" + gpu = seen.get("GPU", "") + if gpu: + cards = [c.strip() for c in gpu.split("|") if c.strip()] + # The count is what admission reads (`gpus`), and the device line is + # written as "N x " when the cards match, which is the other + # spelling the readers infer a count from. Left as a "+"-joined list, + # the row a real two-card box probed to (2026-09-07) had no count at all, + # and admission fell back to job count on a machine it could have gated + # by device. + found["gpus"] = len(cards) + found["device"] = f"{len(cards)} x {cards[0]}" if len(set(cards)) == 1 else " + ".join(cards) + found["kind"] = "gpu" + elif "cores" in found: + found["kind"] = "cpu" + if seen.get("LIBC"): + found["note"] = seen["LIBC"] + return found + + +def write(row: dict[str, Any]) -> Path: + """Append one machine, keeping whatever shape the file already had. + + Refuses rather than rewrites when the file holds entries the reader skips: + serialising the survivors would erase rows the owner wrote by hand, and a + recoverable typo is theirs to fix, not ours to delete on the way past. A + file that does not exist yet is started here -- the first machine an agent + adds for an owner who never ran the command is the common case. + """ + path = connections.store_path() + found = connections.read() + if found.state == UNREADABLE: + raise ValueError(f"{found.detail}\nFix or move that file before adding to it.") + if found.detail: + raise ValueError( + f"{found.detail}\nAdding a machine here would rewrite the file without them. " + "Give those entries an id (or remove them) first, then add this machine." + ) + rows = list(found.rows) + if any(str(r.get("id")).strip() == row["id"] for r in rows): + raise ValueError(f"a machine with id {row['id']!r} is already listed in {path}") + rows.append(row) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps({"connections": rows}, indent=2, ensure_ascii=False) + "\n", encoding="utf-8") + return path + + +ALIAS_MARK = "# raven connection {conn_id} (managed; rewritten on every add)" + + +def write_ssh_alias(row: dict[str, Any]) -> str | None: + """Keep a ``Host `` alias in ``~/.ssh/config`` for an ssh row. + + Transfers cannot ride the exec machine channel -- rsync and scp run their + client HERE and only address the machine -- so without an alias every + transfer carries the raw address, and the raw address then has to live in + the model's context (measured 2026-09-03: a task statement shipped the + ssh string because nothing else could address the machine). With the + alias, ``rsync ... :...`` resolves inside ssh's own config and the + context carries only the registry name. + + One managed block per id, replaced in full on re-add; everything outside + the markers is the owner's and is never touched. Returns the alias, or + None when there is nothing to write (a local row) -- a failure is + reported by the caller as a warning, never as a refusal: the alias is a + convenience beside the registry, not part of it. + """ + if transport_of(row) == LOCAL: + return None + conn_id = str(row["id"]) + mark = ALIAS_MARK.format(conn_id=conn_id) + block = "\n".join( + [ + mark, + f"Host {conn_id}", + f" HostName {row.get('host')}", + f" Port {int(row.get('port') or 22)}", + f" User {row.get('user') or 'root'}", + f" IdentityFile {row.get('key') or '~/.ssh/id_rsa'}", + mark, + ] + ) + path = Path(os.path.expanduser("~/.ssh/config")) + path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + text = path.read_text(encoding="utf-8") if path.exists() else "" + if mark in text: + head, _, rest = text.partition(mark) + _, _, tail = rest.partition(mark) + text = head.rstrip("\n") + ("\n" if head.strip() else "") + tail.lstrip("\n") + text = (text.rstrip("\n") + "\n\n" if text.strip() else "") + block + "\n" + path.write_text(text, encoding="utf-8") + path.chmod(0o600) + return conn_id diff --git a/raven/ops/connections.py b/raven/ops/connections.py index 3f3f4bfb5..1e80952bc 100644 --- a/raven/ops/connections.py +++ b/raven/ops/connections.py @@ -26,13 +26,14 @@ one means a keychain or a master password, and neither is worth building before something asks for it. -Read-only here: the file is written by hand or by -``raven ops connection add`` (``raven/cli/ops_connection_commands.py``). What an -agent is *shown*, and how a campaign is given a way onto the machine it names, -belong to that agent's own installation and are not rendered here; what this -module offers a caller is the store behind the owner's command, the faults that -make a row unusable, and the projection (:func:`shown`) that keeps an address -out of whatever gets handed on. +Read-only here: the file is written by hand or through +:mod:`raven.ops.connection_add`, the one door behind both ``raven ops +connection add`` and the ``ops_connection_add`` tool. What an agent is *shown* +beyond :func:`shown`, and how a campaign is given a way onto the machine it +names, belong to that agent's own installation; what this module offers a +caller is the store behind the owner's command, the faults that make a row +unusable, and the projection (:func:`shown`) that keeps an address out of +whatever gets handed on. """ from __future__ import annotations @@ -277,28 +278,47 @@ def usable(rows: list[dict[str, Any]]) -> list[dict[str, Any]]: # solver belongs on the CPU box" would therefore pick the one machine that # cannot run it. What a machine has installed decides; what it is made of only # narrows. -_SHOWN = ("kind", "device", "gpus", "cores", "memory", "software", "budget_unit", "concurrency", "note") - -# Points this instance at a registry that is not beside its own config. Set by -# a launcher that hosts raven inside another raven: the machines belong to the -# owner, not to whichever sub-agent is asking, and every copy taken to keep an -# instance supplied is a copy that stops being true the day the owner adds a -# machine. Five byte-identical copies existed on this computer on 2026-08-25, -# and the launcher that made them copied once and never again. +SHOWN = ("kind", "device", "gpus", "cores", "memory", "software", "budget_unit", "concurrency", "note") +_SHOWN = SHOWN # the name this module and its tests have always read + +# Points this instance at a registry somewhere else entirely. The machines +# belong to the owner, not to whichever instance is asking, and every copy taken +# to keep an instance supplied is a copy that stops being true the day the owner +# adds a machine. Five byte-identical copies existed on this computer on +# 2026-08-25, and the launcher that made them copied once and never again. CONNECTIONS_ENV = "RAVEN_CONNECTIONS" def store_path() -> Path: - """Where this instance reads its machines: the env var, else beside its config.""" + """Where this instance reads its machines, and where a first add lands. + + The env var when set; else the owner's home (``raven_home()``) when a + registry is there; else the one beside this instance's own config when THAT + exists; else the owner's home, which is where a machine added now belongs. + + The home comes before "beside the config" because a sub-agent runs on a + rendered config of its own, in a state directory of its own, and the host + hands it ``RAVEN_HOME`` -- not a copy of the file. Resolved beside the + config alone, the coding agent read an empty directory while the owner's + machines sat one level up (measured 2026-09-22: ``exec(machine=...)`` + answered "No connection is registered" on a home that listed two), and + the model then reached for the raw address instead. An install that kept + its own list beside a config elsewhere stays on it. + """ override = os.environ.get(CONNECTIONS_ENV, "").strip() if override: return Path(override).expanduser() from raven.config.paths import get_config_path + from raven.home import raven_home + home = raven_home() / STORE + if home.is_file(): + return home try: - return Path(get_config_path()).expanduser().parent / STORE + beside = Path(get_config_path()).expanduser().parent / STORE except Exception: # noqa: BLE001 -- a missing config path is not a failure here - return Path.home() / ".raven" / STORE + return home + return beside if beside.is_file() else home MISSING, UNREADABLE, OK = "missing", "unreadable", "ok" diff --git a/raven/ops/transport.py b/raven/ops/transport.py index 63fbcc9a1..6ea575e1a 100644 --- a/raven/ops/transport.py +++ b/raven/ops/transport.py @@ -138,6 +138,16 @@ def _kill_process_tree(proc: subprocess.Popen, grace_s: float = 2.0) -> None: pass +# The options that make ``-i key`` the ONLY identity ssh may offer. All three +# are needed: ``-i`` is a preference, not a restriction; ``IdentitiesOnly=yes`` +# is defined as excluding what an agent or a provider adds, not what the +# owner's config names; and only reading no config at all (``-F /dev/null``) +# keeps the config's own ``IdentityFile`` lines out. Measured with OpenSSH +# 9.9p2 against a config carrying two IdentityFile lines: ``-i cand`` with +# IdentitiesOnly and no agent still resolved to three identities. +ISOLATE_IDENTITY = ("-F", "/dev/null", "-o", "IdentitiesOnly=yes", "-o", "IdentityAgent=none") + + def make_ssh_runner( host: str, port: int, @@ -145,7 +155,18 @@ def make_ssh_runner( *, user: str = "root", connect_timeout: int = 15, + identities_only: bool = False, ) -> CommandRunner: + """Run a command over ssh. ``identities_only`` narrows auth to ``key`` alone. + + Off by default, which is how work reaches its machine: whatever the owner's + ssh would use gets to work, an agent included. Turned on only where the + session has to prove WHICH key opened it -- the registry probe trying the + keys ssh named when the owner gave no path -- and it takes every option in + :data:`ISOLATE_IDENTITY` to get there. The cost is that an alias or a jump + host in the owner's config is not in play for that one call. + """ + def run(cmd: str) -> tuple[int, str]: argv = [ "ssh", @@ -159,6 +180,10 @@ def run(cmd: str) -> tuple[int, str]: f"ConnectTimeout={connect_timeout}", "-o", "StrictHostKeyChecking=accept-new", + ] + if identities_only: + argv += list(ISOLATE_IDENTITY) + argv += [ f"{user}@{host}", cmd, ] diff --git a/tests/test_agents_code_launcher.py b/tests/test_agents_code_launcher.py index c7c52851f..ce5d867fa 100644 --- a/tests/test_agents_code_launcher.py +++ b/tests/test_agents_code_launcher.py @@ -127,7 +127,17 @@ def get_default_model(self): #: have not boarded (PENDING_WAVE_TOOLS below), so a guide still teaching them #: would promise what the model cannot call. When their wave lands, the #: sections come back with the tools. -GUIDE_MUST_NAME = ("exec", "run_in_background", "read_file", "write_file", "edit_file", "grep", "glob", "todo") +GUIDE_MUST_NAME = ( + "exec", + "run_in_background", + "read_file", + "write_file", + "edit_file", + "grep", + "glob", + "todo", + "ops_connection_add", +) GUIDE_MUST_NOT_NAME = ("exec_write", "exec_read", "job_status", "job_wait", "job_cancel", "cron", "todowrite") CARRIED_GUIDE = RUN_PY.parent / "plugins" / "code-flow" / "prompts" / "TOOLS_CODE.md" CONDUCTS = { @@ -871,6 +881,14 @@ def test_a_relative_state_root_resolves_under_the_host_agent_home(grounded, tmp_ #: the fold off would hold it open at exactly the size it exists for. TRUNK_RESERVED = {"tool_call", "tool_search"} +#: The machine lane, opened 2026-09-23. This product builds a case where the +#: solver is and smoke-tests it there, and in the usual chain it runs before +#: the on-call agent -- so when the owner's registry lists no machine that +#: fits, or nothing at all, the ask and the write have to happen here. Trunk's +#: tool, boarded by ``tools.connectionAdd`` in the product config; the guide's +#: machine section says when to reach for it and when to stay on this box. +MACHINE_LANE_TOOLS = {"ops_connection_add"} + #: The product's visible tool face, hermetically rebuilt from the render: #: the fork's config intent minus the ledgered pending waves, plus the #: opened skill lane and the reserved name above. Trunk also grew tools the @@ -885,6 +903,7 @@ def test_a_relative_state_root_resolves_under_the_host_agent_home(grounded, tmp_ "glob", "grep", "list_dir", + "ops_connection_add", "read_file", "read_skill", "todo", @@ -928,6 +947,7 @@ def test_the_face_arithmetic_is_the_ledger(): (FORK_CONFIG_INTENT - PENDING_WAVE_TOOLS - set(RESPELLED)) | set(RESPELLED.values()) | SKILL_LANE_TOOLS + | MACHINE_LANE_TOOLS | TRUNK_RESERVED ) assert VENDORED_TOOL_FACE == expected diff --git a/tests/test_agents_oncall_flow_connections.py b/tests/test_agents_oncall_flow_connections.py index d4c8be8a5..930e8903b 100644 --- a/tests/test_agents_oncall_flow_connections.py +++ b/tests/test_agents_oncall_flow_connections.py @@ -1,10 +1,11 @@ """The campaign-facing half of the connections suite. -The registry suite itself lives on trunk (tests/test_ops_connections.py, -upstreamed from the fork); these four stayed behind with the campaign -machinery they exercise -- the backend seam reading a campaign's named -connection, the submit path refusing to bury it, and the listing saying -which machine holds the work. +The registry suite itself lives on trunk (tests/test_ops_connections.py); +these stayed behind with the campaign machinery they exercise -- the backend +seam reading a campaign's named connection, the submit path refusing to bury +it, and the listing saying which machine holds the work. The plugin reads the +registry through trunk's module since 2026-09-23, so the store is patched +where the reader lives. """ from __future__ import annotations @@ -22,6 +23,8 @@ from oncall_flow import connections # noqa: E402 from oncall_flow.tools import base as tools_base # noqa: E402 +from raven.ops import connections as trunk # noqa: E402 + @pytest.fixture(autouse=True) def _campaign_root(tmp_path): @@ -31,7 +34,7 @@ def _campaign_root(tmp_path): @pytest.fixture def store(tmp_path, monkeypatch): path = tmp_path / "connections.json" - monkeypatch.setattr(connections, "store_path", lambda: path) + monkeypatch.setattr(trunk, "store_path", lambda: path) def write(rows): path.write_text(json.dumps({"connections": rows}, ensure_ascii=False), encoding="utf-8") @@ -188,41 +191,35 @@ def test_the_listing_of_campaigns_says_which_machine(store, tmp_path, monkeypatc assert "on my CPU box" in out -def test_the_plugin_reader_counts_capacity_the_way_trunk_does(): - """Aligned by hand (the plugin cannot import trunk at runtime); drift is a - parity-ledger entry, so the same cases are pinned on both sides.""" - from oncall_flow import connections as plugin - - row = {"kind": "gpu", "device": "2 x NVIDIA A800-SXM4-80GB", "cores": 128, "memory": "463 GB", "concurrency": 1} - assert plugin.capacity(row) == {"gpus": 2, "cores": 128, "memory_gb": 463} - cpu = {"kind": "cpu", "device": "2 x Intel Xeon Platinum", "cores": 64} - assert plugin.capacity(cpu) == {"cores": 64}, "the N x reading is a GPU row's; a CPU box hands out cores" - assert plugin.resource_unit(cpu) == "cores" - uncounted = {"kind": "gpu", "device": "NVIDIA A800 + NVIDIA A800", "cores": 128} - assert plugin.resource_unit(uncounted) == "", "a GPU row with no device count is gated by job count, not cores" - assert plugin.resource_unit(row) == "gpus" - assert plugin.resource_unit({"cores": 32}) == "cores" - assert plugin.resource_unit({"concurrency": 1}) == "" - assert "gpus" in plugin._SHOWN - said = [str(p) for p in plugin.row_problems({"id": "g", "display_name": "G", "transport": "local", "kind": "gpu"})] - assert any("kind is gpu but neither" in s for s in said) - - -def test_the_plugin_reader_reports_capacity_problems_the_way_trunk_does(): - """Parity for the four doctor behaviours the trunk tests pin: the plugin copy - cannot import trunk, so each is pinned here too, or a mutation in this copy - passes every test (reviewed 2026-09-07).""" - from oncall_flow import connections as plugin - - def said(row): - return [str(p) for p in plugin.row_problems({"id": "g", "display_name": "G", "transport": "local", **row})] - - assert any("'gpus' must be a whole number of devices" in s for s in said({"gpus": 0})) - uncounted = said({"kind": "gpu", "device": "NVIDIA A800 + NVIDIA A800", "cores": 128}) - assert any("kind is gpu but neither 'gpus' nor a device" in s for s in uncounted) - assert any("'concurrency' is not set" in s for s in uncounted), ( - "an uncounted GPU row is gated by job count, so its missing concurrency is reported" - ) - counted = said({"kind": "gpu", "gpus": 2, "concurrency": 1}) - assert any("'concurrency' is not read on a row that says gpus" in s for s in counted) - assert not any("kind is gpu but neither" in s for s in counted) +def test_the_plugin_reader_is_trunks_not_a_copy_of_it(): + """Two readers of one file used to be kept byte-aligned by hand, with a + parity test on each side pinning the same cases; a mutation in the copy + passed every test until the case was re-pinned (reviewed 2026-09-07). One + reader now: the names this plugin serves are trunk's own objects.""" + for name in ( + "capacity", + "resource_unit", + "row_problems", + "usable", + "read", + "load", + "problems", + "shown", + "store_path", + ): + assert getattr(connections, name) is getattr(trunk, name), name + assert connections.SHOWN is trunk.SHOWN + assert connections.LOCAL == trunk.LOCAL and connections.SSH == trunk.SSH + + +def test_what_only_a_campaign_needs_stays_with_the_plugin(store): + """The lookups a campaign makes -- by id, for the owner's name, into a meta + -- are the plugin's; trunk keeps its registry readers to what the CLI and + the exec lane need.""" + store(TWO) + assert connections.get("conn_cpu")["display_name"] == "my CPU box" + assert connections.display_name("conn_cpu") == "my CPU box" + assert connections.display_name("nope") == "nope" + merged = connections.resolve_into({"connection": "conn_cpu", "port": 1}) + assert merged["host"] == TWO[0]["host"] and merged["port"] == 1, "a meta's own value wins over the row" + assert not hasattr(trunk, "resolve_into") diff --git a/tests/test_agents_oncall_flow_watcher.py b/tests/test_agents_oncall_flow_watcher.py index 973276b33..99d22a6b7 100644 --- a/tests/test_agents_oncall_flow_watcher.py +++ b/tests/test_agents_oncall_flow_watcher.py @@ -111,7 +111,6 @@ def test_the_manifest_contributes_the_watcher_service_and_the_ops_tools() -> Non "ops_note", "ops_campaigns", "ops_connections", - "ops_connection_add", "ops_declare", "ops_kill", "ops_outputs", diff --git a/tests/test_agents_oncall_launcher.py b/tests/test_agents_oncall_launcher.py index 29efcc7c7..623e2d5d3 100644 --- a/tests/test_agents_oncall_launcher.py +++ b/tests/test_agents_oncall_launcher.py @@ -53,10 +53,10 @@ def grounded(launcher, tmp_path, monkeypatch): #: moved fails exactly as a silent drift would. The A side is a record and is never #: edited to make this green (tests/fixtures/vendored_fork/README.md). #: -#: ops_connection_add is this side's own tool. The fork never had it: its host -#: filled the connection registry from the owner's answer before dispatching, and -#: that step was removed on 2026-09-06, so the ask now has to name the tool that -#: consumes the answer. +#: ops_connection_add is trunk's tool, served here on tools.connectionAdd. The +#: fork never had it: its host filled the connection registry from the owner's +#: answer before dispatching, and that step was removed on 2026-09-06, so the +#: ask now has to name the tool that consumes the answer. B_SIDE_ADDITIONS = ( ( "for you. A task statement that names no machine is normal.\n", @@ -276,8 +276,11 @@ def test_a_preexisting_own_registry_stays(grounded, tmp_path): #: ops_connection_add is the one row here the fork never had. Its host filled the #: connection registry from the owner's answer before dispatching a spawn; that #: step was removed on 2026-09-06, so the instance that needs the machine now -#: writes the row itself. Listed explicitly, not folded in: a tool reaching this -#: face without a line saying why is the drift this guard exists to catch. +#: writes the row itself. Trunk's tool since 2026-09-23 (raven-code needs the +#: same write, and runs before this product in the usual chain), boarded here by +#: ``tools.connectionAdd`` in the product config rather than by the plugin. +#: Listed explicitly, not folded in: a tool reaching this face without a line +#: saying why is the drift this guard exists to catch. #: #: tool_call is the one name this face carries that no config row put there: #: raven reserves the tool-search meta-pair from tools.disabledTools, because diff --git a/tests/test_agents_oncall_flow_connection_add.py b/tests/test_ops_connection_add.py similarity index 65% rename from tests/test_agents_oncall_flow_connection_add.py rename to tests/test_ops_connection_add.py index 400a41ad1..682a6341f 100644 --- a/tests/test_agents_oncall_flow_connection_add.py +++ b/tests/test_ops_connection_add.py @@ -1,33 +1,29 @@ -"""The on-call instance writes the machine the owner described, after reaching it. +"""An agent writes the machine the owner described, after reaching it. Until 2026-09-06 the host ran `raven ops connection add` on the owner's behalf -before a spawn; that step went with the pre-dispatch registry gate, and an owner who answered the seven questions in conversation had nothing -consuming the answer. These pin the guarantee the write carries over hand-editing --- nothing lands until the machine itself has answered -- and that what lands is -what the listing then shows. +before a spawn; that step went with the pre-dispatch registry gate, and the +write moved into the on-call plugin. It is trunk's since 2026-09-23: the +coding agent builds and smoke-tests where the solver is, and runs BEFORE the +on-call agent, so a registry nobody has written yet has to be writable from +where the need first shows up. These pin the guarantee the write carries over +hand-editing -- nothing lands until the machine itself has answered -- and +that what lands is what the readers then show. """ from __future__ import annotations import asyncio import json -import sys -from pathlib import Path import pytest -REPO = Path(__file__).resolve().parent.parent -PLUGIN_DIR = REPO / "agents" / "raven-oncall" / "plugins" / "oncall-flow" -sys.path.insert(0, str(PLUGIN_DIR)) - -from oncall_flow import connections # noqa: E402 -from oncall_flow.tools import base as tools_base # noqa: E402 -from oncall_flow.tools.ops_connection_add import OpsConnectionAddTool # noqa: E402 +from raven.agent.tools.connection_add import ConnectionAddTool +from raven.ops import connection_add as adder +from raven.ops import connections @pytest.fixture(autouse=True) def _home(tmp_path, monkeypatch): - tools_base.set_home(tmp_path / "ops") monkeypatch.setenv("HOME", str(tmp_path)) monkeypatch.setattr(connections, "store_path", lambda: tmp_path / "connections.json") return tmp_path @@ -35,7 +31,7 @@ def _home(tmp_path, monkeypatch): @pytest.fixture def reached(monkeypatch): - monkeypatch.setattr(connections, "probe", lambda row, **kw: (True, "reached it", {"cores": 32, "kind": "cpu"})) + monkeypatch.setattr(adder, "probe", lambda row, **kw: (True, "reached it", {"cores": 32, "kind": "cpu"})) OWNER_SAID = { @@ -53,7 +49,11 @@ def reached(monkeypatch): def add(**overrides): - return asyncio.run(OpsConnectionAddTool().execute(**{**OWNER_SAID, **overrides})) + return asyncio.run(ConnectionAddTool().execute(**{**OWNER_SAID, **overrides})) + + +def test_the_tool_keeps_the_name_every_guide_and_face_pins(): + assert ConnectionAddTool().name == "ops_connection_add" def test_reached_machine_is_written_and_listed(_home, reached): @@ -62,19 +62,27 @@ def test_reached_machine_is_written_and_listed(_home, reached): rows = json.loads((_home / "connections.json").read_text())["connections"] assert rows[0]["id"] == "my-cpu-box" assert rows[0]["cores"] == 32, "what the machine said about itself is on the row" - assert "my CPU box" in connections.describe() + assert [r["id"] for r in connections.load()] == ["my-cpu-box"], "the readers see it at once" assert "Host my-cpu-box" in (_home / ".ssh" / "config").read_text() + assert "exec's 'machine' parameter" in out, "the id is told where to go, for a product with no ops_declare" + + +def test_a_registry_that_does_not_exist_yet_is_started(_home, reached): + """The common case for an agent: an owner who never ran the command.""" + assert not (_home / "connections.json").exists() + add() + assert (_home / "connections.json").is_file() def test_unreachable_machine_writes_nothing(_home, monkeypatch): - monkeypatch.setattr(connections, "probe", lambda row, **kw: (False, "could not reach it: refused", {})) + monkeypatch.setattr(adder, "probe", lambda row, **kw: (False, "could not reach it: refused", {})) out = add() assert out.startswith("REFUSED") assert not (_home / "connections.json").exists() def test_ssh_without_an_address_is_refused_before_probing(_home, monkeypatch): - monkeypatch.setattr(connections, "probe", lambda row, **kw: pytest.fail("probed with no address")) + monkeypatch.setattr(adder, "probe", lambda row, **kw: pytest.fail("probed with no address")) out = add(host="") assert "REFUSED" in out and "address" in out assert not (_home / "connections.json").exists() @@ -89,7 +97,7 @@ def test_what_the_owner_left_out_comes_from_ssh_config_and_the_key_that_connects stops working the day that agent or that config goes. """ monkeypatch.setattr( - connections, + adder, "ssh_defaults", lambda host, **kw: {"port": 58717, "user": "cfd", "keys": ["~/.ssh/id_ed25519", "~/.ssh/id_rsa"]}, ) @@ -100,7 +108,7 @@ def probe(row, **kw): hit = row["key"] == "~/.ssh/id_rsa" return hit, "reached it" if hit else "refused", {} - monkeypatch.setattr(connections, "probe", probe) + monkeypatch.setattr(adder, "probe", probe) out = add(port=0, user="", key="") assert tried == [("~/.ssh/id_ed25519", True), ("~/.ssh/id_rsa", True)] row = json.loads((_home / "connections.json").read_text())["connections"][0] @@ -108,7 +116,7 @@ def probe(row, **kw): assert "with key ~/.ssh/id_rsa" in out -def test_a_key_the_owner_named_is_probed_the_way_a_job_will_reach_the_machine(_home, monkeypatch): +def test_a_key_the_owner_named_is_probed_the_way_work_will_reach_the_machine(_home, monkeypatch): """No isolation for a path the owner asserted: an agent doing the authenticating is their setup working, and refusing it here would refuse a machine every later job can reach.""" @@ -118,12 +126,29 @@ def probe(row, **kw): seen.append(kw.get("isolate_key")) return True, "reached it", {} - monkeypatch.setattr(connections, "probe", probe) - monkeypatch.setattr(connections, "ssh_defaults", lambda host, **kw: pytest.fail("resolved a key the owner gave")) + monkeypatch.setattr(adder, "probe", probe) + monkeypatch.setattr(adder, "ssh_defaults", lambda host, **kw: pytest.fail("resolved a key the owner gave")) add(key="~/.ssh/id_rsa") assert seen == [False] +def test_the_probe_rides_the_transport_exec_uses(monkeypatch): + """The machine is reached here the way `exec(machine=...)` will reach it + afterwards -- one seam, so a row that probes reachable is one that runs.""" + from raven.ops import transport + + asked = [] + + def runner_from(row, *, cap_seconds=None): + asked.append((row["id"], cap_seconds)) + return lambda cmd: (0, "CORES=8\n") + + monkeypatch.setattr(transport, "runner_from", runner_from) + reached, message, found = adder.probe({"id": "box", "transport": "local"}, timeout=7) + assert reached and found == {"cores": 8, "kind": "cpu"} + assert asked == [("box", 7)] + + def test_an_isolated_candidate_leaves_ssh_exactly_one_identity_to_offer(tmp_path, monkeypatch): """Measured through ssh's own resolver, because the argv does not settle it. @@ -141,7 +166,7 @@ def test_an_isolated_candidate_leaves_ssh_exactly_one_identity_to_offer(tmp_path import shutil import subprocess - from oncall_flow.docker_backend import make_ssh_runner + from raven.ops.transport import make_ssh_runner if not shutil.which("ssh"): pytest.skip("no ssh on this computer") @@ -192,3 +217,19 @@ def as_ssh_reads_it(*, isolated: bool) -> list[str]: ] ) assert str(other_a) in agent_only and str(other_b) in agent_only + + +def test_the_ask_is_one_text_the_on_call_listing_and_the_tool_share(): + """Spelled once: the on-call plugin's empty-registry reply carries trunk's + ASK_OWNER, so the questions cannot drift between the two products.""" + import sys + from pathlib import Path + + plugin_dir = Path(__file__).resolve().parent.parent / "agents" / "raven-oncall" / "plugins" / "oncall-flow" + sys.path.insert(0, str(plugin_dir)) + try: + from oncall_flow import connections as plugin + finally: + sys.path.remove(str(plugin_dir)) + assert adder.ASK_OWNER in plugin._REQUEST + assert "ops_connection_add" in adder.ASK_OWNER diff --git a/tests/test_ops_connections.py b/tests/test_ops_connections.py index 914bb4b41..ae3116ecc 100644 --- a/tests/test_ops_connections.py +++ b/tests/test_ops_connections.py @@ -368,3 +368,80 @@ def test_a_gpu_row_with_no_device_count_is_admitted_by_job_count_not_by_cores(): assert connections.resource_unit(row) == "" said = [str(p) for p in connections.row_problems({"id": "g", "display_name": "G", "transport": "local", **row})] assert any("admitted by job count" in s for s in said) + + +# ---- where the registry is looked for -------------------------------------- + + +def _rendered_config(tmp_path, monkeypatch): + """A sub-agent's shape: its own config in a state directory of its own, and + the host's home handed down as RAVEN_HOME.""" + import raven.home as home + + state = tmp_path / "state" + state.mkdir() + rendered = state / ".config.rendered.json" + rendered.write_text("{}") + monkeypatch.setattr(home, "_current_config_path", rendered) + monkeypatch.setenv("RAVEN_HOME", str(tmp_path / "home")) + monkeypatch.delenv(connections.CONNECTIONS_ENV, raising=False) + (tmp_path / "home").mkdir() + return state + + +def test_a_sub_agent_reads_the_owners_registry_not_an_empty_directory_of_its_own(tmp_path, monkeypatch): + """Measured 2026-09-22: Raven-Code, on a rendered config in its state root, + resolved the registry beside that config, found nothing, and reported "No + connection is registered" on a home that listed two machines -- then reached + for the raw ssh address. The home the host hands down is where the owner's + machines are.""" + _rendered_config(tmp_path, monkeypatch) + theirs = tmp_path / "home" / "connections.json" + theirs.write_text(json.dumps({"connections": [{"id": "conn_cpu", "display_name": "CPU", "transport": "local"}]})) + + assert connections.store_path() == theirs + assert [r["id"] for r in connections.load()] == ["conn_cpu"] + + +def test_a_first_add_lands_in_the_owners_home_when_nothing_exists_yet(tmp_path, monkeypatch): + """Where the write goes decides who reads it afterwards: the coding agent + adds a machine before the on-call agent runs, and the on-call agent reads + the owner's home. A row written into the coding agent's own state root + would be a registry nobody else can see.""" + state = _rendered_config(tmp_path, monkeypatch) + assert not (state / "connections.json").exists() and not (tmp_path / "home" / "connections.json").exists() + + assert connections.store_path() == tmp_path / "home" / "connections.json" + + +def test_an_install_that_kept_its_own_list_beside_its_config_stays_on_it(tmp_path, monkeypatch): + """Nothing in the owner's home, a registry beside the config: that is an + install that predates this resolution and wrote its list where the old rule + read it. It keeps working unchanged.""" + state = _rendered_config(tmp_path, monkeypatch) + own = state / "connections.json" + own.write_text(json.dumps({"connections": [{"id": "mine", "display_name": "Mine", "transport": "local"}]})) + + assert connections.store_path() == own + assert [r["id"] for r in connections.load()] == ["mine"] + + +def test_the_owners_home_wins_over_a_list_beside_the_config_when_both_exist(tmp_path, monkeypatch): + """Two lists, one truth: the owner's. A copy in a state directory is the + copy-once bug -- five byte-identical registries on one computer, 2026-08-25 + -- and reading it would hide every machine the owner added since.""" + state = _rendered_config(tmp_path, monkeypatch) + (state / "connections.json").write_text(json.dumps({"connections": []})) + theirs = tmp_path / "home" / "connections.json" + theirs.write_text(json.dumps({"connections": [{"id": "conn_gpu", "display_name": "GPU", "transport": "local"}]})) + + assert connections.store_path() == theirs + + +def test_the_env_var_still_outranks_every_default(tmp_path, monkeypatch): + _rendered_config(tmp_path, monkeypatch) + (tmp_path / "home" / "connections.json").write_text(json.dumps({"connections": []})) + elsewhere = tmp_path / "elsewhere.json" + monkeypatch.setenv(connections.CONNECTIONS_ENV, str(elsewhere)) + + assert connections.store_path() == elsewhere From 4a71d93120dd3be38ac4be54c1f744b300b81706 Mon Sep 17 00:00:00 2001 From: "xiaotian.luo" Date: Wed, 23 Sep 2026 12:43:03 +0800 Subject: [PATCH 2/6] fix(ops): give the ssh probe a deadline again, and cover the tool's refusals The shared probe passed its 30 s timeout to `runner_from`, which applied it only to the local runner; the ssh runner called `subprocess.run` with no timeout, and `ConnectTimeout` is over once the session is up. A machine that accepted and then stopped answering held `raven ops connection add` (and the tool's worker thread) forever, where the old CLI gave up at 30 s (review on #690). `make_ssh_runner` takes `cap_seconds`, returns `TIMED_OUT_RC` when it fires (the code the local runner already used), and `runner_from` passes the cap to both runners. The probe reads 124 as "no answer within Ns" on both of its ssh paths. The machine channel now gets its own backstop on ssh too, behind the wrapper that caps the far side. Tests drive a stand-in ssh on PATH for the silent, refusing and missing cases, and cover each refusal and remark the tool can return; diff coverage on the branch measures 99%. Co-authored-by: Claude (claude-opus-5-5) --- raven/ops/connection_add.py | 7 +- raven/ops/transport.py | 18 +++- tests/test_ops_connection_add.py | 148 +++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+), 2 deletions(-) diff --git a/raven/ops/connection_add.py b/raven/ops/connection_add.py index 9bb6f2c43..ed3aefe10 100644 --- a/raven/ops/connection_add.py +++ b/raven/ops/connection_add.py @@ -113,7 +113,7 @@ def probe(row: dict[str, Any], *, timeout: float = 30.0, isolate_key: bool = Fal authenticated, and the registry would then hold a path that stops working the day that agent or config changes. """ - from raven.ops.transport import TransportError, make_ssh_runner, runner_from + from raven.ops.transport import TIMED_OUT_RC, TransportError, make_ssh_runner, runner_from where = "this computer" if transport_of(row) == LOCAL else f"{row.get('user')}@{row.get('host')}:{row.get('port')}" try: @@ -124,6 +124,7 @@ def probe(row: dict[str, Any], *, timeout: float = 30.0, isolate_key: bool = Fal os.path.expanduser(str(row.get("key") or "")), user=str(row.get("user") or "root"), identities_only=True, + cap_seconds=timeout, ) else: runner = runner_from(row, cap_seconds=timeout) @@ -132,6 +133,10 @@ def probe(row: dict[str, Any], *, timeout: float = 30.0, isolate_key: bool = Fal return False, f"could not reach {where}: {exc}", {} except FileNotFoundError: return False, f"could not reach {where}: ssh not found on this computer", {} + if code == TIMED_OUT_RC: + # Reached and then silent, or never answering at all: either way a row + # is not written on it, and the owner is told how long was waited. + return False, f"could not reach {where}: no answer within {timeout:.0f}s", {} if code != 0: return False, f"could not reach {where}: {out.strip() or f'exit {code}'}", {} return True, f"reached {where}", parse_probe(out) diff --git a/raven/ops/transport.py b/raven/ops/transport.py index 6ea575e1a..6c176983c 100644 --- a/raven/ops/transport.py +++ b/raven/ops/transport.py @@ -156,9 +156,20 @@ def make_ssh_runner( user: str = "root", connect_timeout: int = 15, identities_only: bool = False, + cap_seconds: float | None = None, ) -> CommandRunner: """Run a command over ssh. ``identities_only`` narrows auth to ``key`` alone. + ``cap_seconds`` bounds the whole call, not only the connect: a machine that + accepts the session and then stops answering held the caller forever, + because ``ConnectTimeout`` is over once the connection is up (reviewed + 2026-09-23 -- the registry probe lost the 30 s the old CLI enforced when + it moved onto this runner). A cap that fires returns ``TIMED_OUT_RC`` with + whatever arrived, the code the local runner uses, so a caller reads one + code however the machine is reached. The local ssh client is what gets + killed; bounding the far side is the caller's to do (the machine channel + wraps its command for that). + Off by default, which is how work reaches its machine: whatever the owner's ssh would use gets to work, an agent included. Turned on only where the session has to prove WHICH key opened it -- the registry probe trying the @@ -187,7 +198,11 @@ def run(cmd: str) -> tuple[int, str]: f"{user}@{host}", cmd, ] - proc = subprocess.run(argv, capture_output=True, text=True) + try: + proc = subprocess.run(argv, capture_output=True, text=True, timeout=cap_seconds) + except subprocess.TimeoutExpired as exc: + got = exc.stdout.decode(errors="replace") if isinstance(exc.stdout, bytes) else (exc.stdout or "") + return TIMED_OUT_RC, got tail = f"\n{proc.stderr}" if proc.stderr and proc.returncode != 0 else "" return proc.returncode, proc.stdout + tail @@ -223,4 +238,5 @@ def runner_from(row: dict[str, Any], *, cap_seconds: float | None = None) -> Com # A connection names the account to log in as. Dropping it would make # that field one more setting that is written, accepted and does nothing. user=str(row.get("user") or "root"), + cap_seconds=cap_seconds, ) diff --git a/tests/test_ops_connection_add.py b/tests/test_ops_connection_add.py index 682a6341f..51830b066 100644 --- a/tests/test_ops_connection_add.py +++ b/tests/test_ops_connection_add.py @@ -21,6 +21,9 @@ from raven.ops import connection_add as adder from raven.ops import connections +# Taken before the autouse fixture below replaces it for every test. +_REAL_STORE_PATH = connections.store_path + @pytest.fixture(autouse=True) def _home(tmp_path, monkeypatch): @@ -233,3 +236,148 @@ def test_the_ask_is_one_text_the_on_call_listing_and_the_tool_share(): sys.path.remove(str(plugin_dir)) assert adder.ASK_OWNER in plugin._REQUEST assert "ops_connection_add" in adder.ASK_OWNER + + +# ---- the probe, through a stand-in ssh on PATH ------------------------------ + + +def _fake_ssh(tmp_path, monkeypatch, body: str): + """Put an executable named ssh first on PATH; the runners call it by name.""" + bin_dir = tmp_path / "bin" + bin_dir.mkdir(exist_ok=True) + script = bin_dir / "ssh" + script.write_text("#!/bin/sh\n" + body + "\n") + script.chmod(0o755) + monkeypatch.setenv("PATH", f"{bin_dir}:/usr/bin:/bin") + + +SSH_ROW = {"id": "box", "transport": "ssh", "host": "203.0.113.7", "port": 58717, "user": "root", "key": "~/k"} + + +@pytest.mark.parametrize("isolate_key", [False, True], ids=["as-work-reaches-it", "candidate-key-alone"]) +def test_a_machine_that_answers_and_then_goes_silent_is_given_up_on(tmp_path, monkeypatch, isolate_key): + """The reviewed hole (2026-09-23): ConnectTimeout is over once the session + is up, and the ssh runner had no deadline of its own, so a probe against a + machine that accepted and then stopped answering never returned. The old + CLI capped the call at 30 s; the shared probe keeps that cap on both of + its ssh paths, and nothing is written on a machine that went quiet.""" + import time + + _fake_ssh(tmp_path, monkeypatch, "sleep 30") + started = time.monotonic() + reached, message, found = adder.probe(dict(SSH_ROW), timeout=0.5, isolate_key=isolate_key) + assert time.monotonic() - started < 10 + assert (reached, found) == (False, {}) + assert "no answer within" in message + + +def test_a_machine_that_refuses_says_what_ssh_said(tmp_path, monkeypatch): + _fake_ssh(tmp_path, monkeypatch, "echo 'Permission denied (publickey).' >&2; exit 255") + reached, message, _ = adder.probe(dict(SSH_ROW), timeout=5) + assert not reached and "Permission denied" in message + + +def test_no_ssh_client_on_this_computer_is_a_refusal_not_a_crash(tmp_path, monkeypatch): + empty = tmp_path / "empty" + empty.mkdir() + monkeypatch.setenv("PATH", str(empty)) + reached, message, _ = adder.probe(dict(SSH_ROW), timeout=5) + assert not reached and "ssh not found" in message + + +def test_a_row_with_no_address_is_refused_by_the_transport(): + reached, message, _ = adder.probe({"id": "box", "transport": "ssh", "port": 22}, timeout=5) + assert not reached and "no address" in message + + +def test_ssh_defaults_reads_what_ssh_resolves_and_keeps_only_keys_that_exist(tmp_path, monkeypatch): + real = tmp_path / "id_real" + real.write_text("") + _fake_ssh( + tmp_path, + monkeypatch, + f"printf 'user cfd\\nport 58717\\nidentityfile {real}\\nidentityfile {tmp_path}/id_gone\\n'", + ) + assert adder.ssh_defaults("203.0.113.7") == {"user": "cfd", "port": 58717, "keys": [str(real)]} + + +def test_ssh_defaults_is_empty_when_ssh_cannot_answer(tmp_path, monkeypatch): + _fake_ssh(tmp_path, monkeypatch, "exit 255") + assert adder.ssh_defaults("203.0.113.7", port=22, user="root") == {} + + +def test_the_probe_reads_the_c_library_line_into_the_note(): + assert adder.parse_probe("CORES=8\nLIBC=ldd (GNU libc) 2.31\n")["note"] == "ldd (GNU libc) 2.31" + + +# ---- the tool's refusals and remarks -------------------------------------- + + +def test_a_machine_with_no_name_is_refused(): + assert add(name=" ").startswith("REFUSED") + + +def test_a_local_machine_is_probed_here_and_written_without_an_alias(_home, reached): + out = add(transport="local", host="", note="the lab mac") + row = json.loads((_home / "connections.json").read_text())["connections"][0] + assert row["transport"] == "local" and row["note"] == "the lab mac" + assert "ssh alias" not in out + + +def test_no_key_to_try_is_a_question_for_the_owner(_home, monkeypatch): + monkeypatch.setattr(adder, "ssh_defaults", lambda host, **kw: {"keys": []}) + out = add(key="") + assert out.startswith("REFUSED") and "key path" in out + + +def test_candidates_that_all_fail_name_the_keys_tried_and_why_an_alias_is_out(_home, monkeypatch): + monkeypatch.setattr(adder, "ssh_defaults", lambda host, **kw: {"keys": ["~/.ssh/a", "~/.ssh/b"]}) + monkeypatch.setattr(adder, "probe", lambda row, **kw: (False, "refused", {})) + out = add(key="") + assert out.startswith("REFUSED") and "~/.ssh/a, ~/.ssh/b" in out and "jump host" in out + + +def test_a_row_the_readers_could_not_use_is_not_written(_home, reached): + out = add(id="Not An Id") + assert out.startswith("REFUSED") and "lowercase" in out + assert not (_home / "connections.json").exists() + + +def test_an_id_already_listed_is_refused_rather_than_overwritten(_home, reached): + add() + out = add() + assert out.startswith("REFUSED") and "already listed" in out + + +def test_what_is_worth_the_owners_attention_is_said_without_refusing(_home, reached): + out = add(software="") + assert "wrote my-cpu-box" in out and "Worth telling the owner" in out and "'software' is not set" in out + + +def test_concurrency_is_kept_only_where_nothing_else_says_how_many_jobs_fit(_home, monkeypatch): + monkeypatch.setattr(adder, "probe", lambda row, **kw: (True, "reached it", {})) + add(concurrency=3) + assert json.loads((_home / "connections.json").read_text())["connections"][0]["concurrency"] == 3 + + +def test_an_alias_that_cannot_be_written_is_a_warning_not_a_refusal(_home, reached, monkeypatch): + def refuse(row): + raise OSError("read-only file system") + + monkeypatch.setattr(adder, "write_ssh_alias", refuse) + out = add() + assert "wrote my-cpu-box" in out and "ssh alias not written (read-only file system)" in out + + +def test_with_no_config_path_to_stand_beside_the_home_is_the_answer(tmp_path, monkeypatch): + """The autouse fixture stands a scratch store in; this one needs the real + resolution, so it calls the function the fixture replaced.""" + import raven.config.paths as paths + + def no_config(): + raise RuntimeError("no config") + + monkeypatch.setattr(paths, "get_config_path", no_config) + monkeypatch.setenv("RAVEN_HOME", str(tmp_path / "home")) + monkeypatch.delenv(connections.CONNECTIONS_ENV, raising=False) + assert _REAL_STORE_PATH() == tmp_path / "home" / "connections.json" From a525fcf9a4e340b080ad9c7a25532055a6a4cf18 Mon Sep 17 00:00:00 2001 From: "xiaotian.luo" Date: Thu, 24 Sep 2026 16:51:10 +0800 Subject: [PATCH 3/6] fix(agent): let exec's machine lane follow the registry, and keep a config's own list Review findings on the registry-writer PR (2026-09-24). Blocker: a process that started with no machine kept an `exec` schema without `machine` after the first `ops_connection_add`, because the registry serves a tool from the copy it took at admission and `ExecTool` authored no `to_schema` -- so the reply, the typed-ssh refusal from #552 and the new Raven-Code guide all sent the model to `machine=` while the schema it was shown had no such parameter. `ExecTool.to_schema` is authored now, which is how a tool declares a dynamic shape; the shape moves only when the registry goes from empty to not. The registry falls back to the owner's home only when there is none beside the instance's own config, in `raven.ops.connections.store_path` and in the on-call launcher's `connections_registry`. Home-first was right for a sub-agent, whose config sits in a state directory nothing writes, but it silently moved a `--config /x/config.json` host off `/x/connections.json` as soon as a registry appeared in the home -- which an agent's first add now writes with no owner action. A sub-agent still resolves the home: nothing is ever written beside its rendered config. Also: `ops_connection_add` joins the settings tool table's `run` group, and the guard that diffs that table against a loop's registry turns the flag on, so it can see opt-in tools; `CONTEXT.md`'s Machine entry and both on-call doc pages describe the lookup and the tool; the plugin's orphaned `_PROBE` is gone; and #552's ssh-version test asks the client whether `-P` takes a value instead of looking for the string "tag", which an older client prints as the hostname -- so it never skipped on OpenSSH 8.9. Co-authored-by: Claude (claude-opus-5-5) --- CHANGELOG.md | 7 +-- CONTEXT.md | 23 +++++---- .../oncall-flow/oncall_flow/connections.py | 11 ----- agents/raven-oncall/run.py | 10 ++-- docs-site/docs/oncall.md | 7 +-- docs-site/docs/oncall.zh.md | 5 +- raven/agent/tools/machine_exec.py | 3 +- raven/agent/tools/shell.py | 19 ++++++- raven/ops/connections.py | 32 ++++++------ tests/test_agents_oncall_launcher.py | 13 +++++ tests/test_ops_connections.py | 23 ++++++--- tests/test_settings_tool_groups.py | 3 ++ tests/test_shell_machine_channel.py | 49 +++++++++++++++++-- ui-web/src/features/settings/toolGroups.json | 3 +- 14 files changed, 143 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01c3ff22e..941e4643b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,9 +34,10 @@ All notable changes to Raven are documented here. ### Changed -- The connection registry resolves to the owner's home first - (`raven_home()/connections.json`), then beside the instance's own config, - then the home again for a first write. A sub-agent runs on a rendered +- The connection registry falls back to the owner's home + (`raven_home()/connections.json`) when there is none beside the instance's + own config, and a first write lands there. A list beside the config still + comes first, so a `--config` host keeps its own file. A sub-agent runs on a rendered config in a state directory of its own and is handed `RAVEN_HOME`, not a copy of the file; resolved beside the config alone, Raven-Code read an empty directory while the owner's machines sat one level up, and reached diff --git a/CONTEXT.md b/CONTEXT.md index 1a577b7d3..37a226356 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -337,16 +337,19 @@ agents, and nobody registered them; "builtin" — that is the in-process row, wh no subprocess and no launcher. **Machine** (`raven/ops/connections.py`): -A compute host the owner registered with `raven ops connection add`, held in -`connections.json` beside the config or wherever `RAVEN_CONNECTIONS` points. -An on-call-style agent runs its work outside the dispatching Raven -process — on a GPU box, a lab workstation, another machine entirely — and the -host's whole part in that is keeping the registry and handing it over: a -launcher points the agent's `RAVEN_CONNECTIONS` at the owner's store rather -than copying rows into the agent's own home. Which machine a job lands on, -and whether it can run at all, is settled inside the agent that runs it. -Nothing on the dispatch path reads the registry, so no graph and no spawn is -ever refused over the state of it. +A compute host the owner registered — with `raven ops connection add`, or, +since 2026-09-23, with the `ops_connection_add` tool an agent drives from the +owner's answers in conversation. The registry is the path `RAVEN_CONNECTIONS` +points at, else `connections.json` beside the active config, else the one in +the raven home, where a first registration lands. An on-call-style agent runs +its work outside the dispatching Raven process — on a GPU box, a lab +workstation, another machine entirely — and the host's whole part in that is +keeping the registry and handing it over: a launcher points the agent's +`RAVEN_CONNECTIONS` at the owner's store rather than copying rows into the +agent's own home, and a sub-agent inherits `RAVEN_HOME` so it resolves the +same file. Which machine a job lands on, and whether it can run at all, is +settled inside the agent that runs it. Nothing on the dispatch path reads the +registry, so no graph and no spawn is ever refused over the state of it. _Avoid_: "host" / "server" / "node" (too broad, no link to the `ops connection` registry that supplies the rows); "GPU box" (only some are GPU hosts, and the term covers any registered compute destination); diff --git a/agents/raven-oncall/plugins/oncall-flow/oncall_flow/connections.py b/agents/raven-oncall/plugins/oncall-flow/oncall_flow/connections.py index d0ca730ae..6402741b8 100644 --- a/agents/raven-oncall/plugins/oncall-flow/oncall_flow/connections.py +++ b/agents/raven-oncall/plugins/oncall-flow/oncall_flow/connections.py @@ -200,14 +200,3 @@ def describe() -> str: "machine time, and coming back is ops_check_later. A cron job and a file " "of your own do the same arithmetic with none of the record." ) - - -# What a machine is asked about itself, the moment it is reached. Read off the -# box rather than typed by the owner, so the row cannot say 64 cores about a -# machine with 32. Mirrors the CLI's probe line. -_PROBE = ( - "echo CORES=$(nproc 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null); " - "echo MEM=$(free -g 2>/dev/null | awk '/^Mem:/{print $2}'); " - "echo GPU=$(nvidia-smi --query-gpu=name,memory.total --format=csv,noheader 2>/dev/null | paste -sd'|' -); " - "echo LIBC=$(ldd --version 2>/dev/null | head -1)" -) diff --git a/agents/raven-oncall/run.py b/agents/raven-oncall/run.py index 863a8db6d..a576549e1 100644 --- a/agents/raven-oncall/run.py +++ b/agents/raven-oncall/run.py @@ -117,13 +117,13 @@ def connections_registry() -> Path: would see no machines at all, and a task naming a remote path would be read as naming a local one. A pointer rather than a copy: a copy is wrong from the first machine the owner adds, with nothing to say so. An install - that predates this and keeps its own list stays on it. + that predates this and keeps its own list stays on it -- checked first, so a + home registry appearing later (an agent's first add writes one) does not + quietly take its place (reviewed 2026-09-24: home-first dropped such a list + at the next launch, both rows still on disk and unread). """ - host = raven_home() / "connections.json" - if host.is_file(): - return host own = state_root() / "connections.json" - return own if own.is_file() else host + return own if own.is_file() else raven_home() / "connections.json" def seed_guide(workspace: Path) -> None: diff --git a/docs-site/docs/oncall.md b/docs-site/docs/oncall.md index 8545aa5c7..30f8510dd 100644 --- a/docs-site/docs/oncall.md +++ b/docs-site/docs/oncall.md @@ -50,9 +50,10 @@ raven ops connection add --id local-lab --name "Local lab" \ --non-interactive ``` -Registration writes `connections.json` in the raven home (`RAVEN_HOME`, or -`~/.raven`), or the path selected by `RAVEN_CONNECTIONS`. A registry beside a -config kept elsewhere is still read when the home has none. Sub-agents run on a +The registry is the path selected by `RAVEN_CONNECTIONS`; else the +`connections.json` beside the active config, when there is one; else the one in +the raven home (`RAVEN_HOME`, or `~/.raven`), which is also where a first +registration lands. Sub-agents run on a rendered config in a state directory of their own and inherit the home, so they read the owner's registry rather than a copy of it. SSH registration can also write a managed alias in `~/.ssh/config`; the key path is recorded, not diff --git a/docs-site/docs/oncall.zh.md b/docs-site/docs/oncall.zh.md index dd5d42653..bd4d9e0f7 100644 --- a/docs-site/docs/oncall.zh.md +++ b/docs-site/docs/oncall.zh.md @@ -41,8 +41,9 @@ raven ops connection add --id local-lab --name "Local lab" \ --non-interactive ``` -注册会写入 raven 家目录（`RAVEN_HOME`，默认 `~/.raven`）下的 `connections.json`， -或 `RAVEN_CONNECTIONS` 指定位置；家目录里没有时，仍会读放在别处 config 旁的那份。 +注册表的位置:有 `RAVEN_CONNECTIONS` 就用它指的文件;否则当前 config 旁边有 +`connections.json` 就用那份;都没有时用 raven 家目录(`RAVEN_HOME`,默认 `~/.raven`)下的那份, +第一次注册也写在这里。 子代理各自用渲染出的 config 跑在自己的状态目录里、继承家目录，所以读到的是 主人的注册表，不是一份副本。SSH 注册还可能在 `~/.ssh/config` 写入受管理的别名； 只记录私钥路径，不记录私钥内容。不要把密钥或密码放进任务消息。 diff --git a/raven/agent/tools/machine_exec.py b/raven/agent/tools/machine_exec.py index a6a935567..d9a2a9529 100644 --- a/raven/agent/tools/machine_exec.py +++ b/raven/agent/tools/machine_exec.py @@ -87,7 +87,8 @@ def machines_registered() -> bool: """Whether this install has any connection to offer the channel for. Read per schema build rather than cached: the registry can be written - mid-session, and the loop rebuilds the tool schema every turn. + mid-session, and ``ExecTool.to_schema`` has the tool registry render + exec's schema per model call rather than from its registration copy. """ try: from raven.ops.connections import load diff --git a/raven/agent/tools/shell.py b/raven/agent/tools/shell.py index 83cdb7c2a..ec8ec548c 100644 --- a/raven/agent/tools/shell.py +++ b/raven/agent/tools/shell.py @@ -119,12 +119,27 @@ def name(self) -> str: # that does not exist under that name. _GLOB_CHARS = frozenset("*?[") + def to_schema(self) -> dict[str, Any]: + """This call's shape, rendered fresh for every model call. + + Authored rather than inherited on purpose: ``ToolRegistry`` serves a + tool from the copy it took at registration unless the tool writes its + own ``to_schema``, and the machine lane below depends on the registry, + which can be written mid-session. Served from that copy, a process that + started with no machine kept an ``exec`` without ``machine`` after the + first ``ops_connection_add`` -- while that tool's reply, the typed-ssh + refusal and the coding guide all sent the model to ``machine=`` + (reviewed 2026-09-24). The shape moves only when the registry goes from + empty to not, so the array stays stable across calls otherwise. + """ + return super().to_schema() + @property def description(self) -> str: # Two descriptions, because the second sentence is a lie for an owner # who registered no machines. Read per call rather than fixed at - # construction -- the registry can be written mid-session, and the loop - # rebuilds the tool schema every turn. + # construction -- the registry can be written mid-session, and + # ``to_schema`` above is what makes the registry serve this live. base = "Execute a shell command and return its output. Use with caution." from raven.agent.tools.machine_exec import machines_registered diff --git a/raven/ops/connections.py b/raven/ops/connections.py index 1e80952bc..942d97b97 100644 --- a/raven/ops/connections.py +++ b/raven/ops/connections.py @@ -292,18 +292,23 @@ def usable(rows: list[dict[str, Any]]) -> list[dict[str, Any]]: def store_path() -> Path: """Where this instance reads its machines, and where a first add lands. - The env var when set; else the owner's home (``raven_home()``) when a - registry is there; else the one beside this instance's own config when THAT - exists; else the owner's home, which is where a machine added now belongs. - - The home comes before "beside the config" because a sub-agent runs on a - rendered config of its own, in a state directory of its own, and the host - hands it ``RAVEN_HOME`` -- not a copy of the file. Resolved beside the - config alone, the coding agent read an empty directory while the owner's - machines sat one level up (measured 2026-09-22: ``exec(machine=...)`` - answered "No connection is registered" on a home that listed two), and - the model then reached for the raw address instead. An install that kept - its own list beside a config elsewhere stays on it. + The env var when set; else the registry beside this instance's own config + when one is there; else the owner's home (``raven_home()``), which is also + where a machine added now belongs. + + The home is the fallback rather than a plain "beside the config" because a + sub-agent runs on a rendered config in a state directory of its own, where + nothing is ever written, and the host hands it ``RAVEN_HOME`` -- not a copy + of the file. Resolved beside the config alone, the coding agent read that + empty directory while the owner's machines sat in the home (measured + 2026-09-22: ``exec(machine=...)`` answered "No connection is registered" on + a home that listed two), and the model reached for the raw address instead. + + A list beside the config comes first so that one never moves: a host started + with ``--config /x/config.json`` beside its own ``/x/connections.json`` + keeps reading that file after a registry first appears in the home -- which + an agent's first add now writes without the owner doing anything (reviewed + 2026-09-24: home-first switched such a host to the new file, silently). """ override = os.environ.get(CONNECTIONS_ENV, "").strip() if override: @@ -312,15 +317,12 @@ def store_path() -> Path: from raven.home import raven_home home = raven_home() / STORE - if home.is_file(): - return home try: beside = Path(get_config_path()).expanduser().parent / STORE except Exception: # noqa: BLE001 -- a missing config path is not a failure here return home return beside if beside.is_file() else home - MISSING, UNREADABLE, OK = "missing", "unreadable", "ok" diff --git a/tests/test_agents_oncall_launcher.py b/tests/test_agents_oncall_launcher.py index 623e2d5d3..cb660a6f9 100644 --- a/tests/test_agents_oncall_launcher.py +++ b/tests/test_agents_oncall_launcher.py @@ -259,6 +259,19 @@ def test_a_preexisting_own_registry_stays(grounded, tmp_path): assert grounded.connections_registry() == own +def test_an_own_registry_stays_after_a_home_registry_appears(grounded, tmp_path): + """An agent's first add writes a home registry with no owner action; an + install that kept its own list must not drop it at the next launch + (reviewed 2026-09-24).""" + own = tmp_path / "state" / "connections.json" + own.parent.mkdir(parents=True, exist_ok=True) + own.write_text("[]") + home = tmp_path / "home" + home.mkdir(parents=True, exist_ok=True) + (home / "connections.json").write_text("[]") + assert grounded.connections_registry() == own + + # --- the visible tool face ---------------------------------------------------- diff --git a/tests/test_ops_connections.py b/tests/test_ops_connections.py index ae3116ecc..e75a2cc41 100644 --- a/tests/test_ops_connections.py +++ b/tests/test_ops_connections.py @@ -426,16 +426,23 @@ def test_an_install_that_kept_its_own_list_beside_its_config_stays_on_it(tmp_pat assert [r["id"] for r in connections.load()] == ["mine"] -def test_the_owners_home_wins_over_a_list_beside_the_config_when_both_exist(tmp_path, monkeypatch): - """Two lists, one truth: the owner's. A copy in a state directory is the - copy-once bug -- five byte-identical registries on one computer, 2026-08-25 - -- and reading it would hide every machine the owner added since.""" +def test_a_list_beside_the_config_keeps_winning_after_a_home_registry_appears(tmp_path, monkeypatch): + """The reviewed shape (2026-09-24): a host started with ``--config`` beside + its own list. An agent's first add now writes a home registry with no + owner action, and home-first switched that host to the new file -- its own + machines still on disk, no longer read. The list beside the config stays + the answer.""" state = _rendered_config(tmp_path, monkeypatch) - (state / "connections.json").write_text(json.dumps({"connections": []})) - theirs = tmp_path / "home" / "connections.json" - theirs.write_text(json.dumps({"connections": [{"id": "conn_gpu", "display_name": "GPU", "transport": "local"}]})) + own = state / "connections.json" + own.write_text(json.dumps({"connections": [{"id": "conn_cpu_32c", "display_name": "CPU", "transport": "local"}]})) + assert connections.store_path() == own - assert connections.store_path() == theirs + (tmp_path / "home" / "connections.json").write_text( + json.dumps({"connections": [{"id": "conn_new", "display_name": "New", "transport": "local"}]}) + ) + + assert connections.store_path() == own + assert [r["id"] for r in connections.load()] == ["conn_cpu_32c"] def test_the_env_var_still_outranks_every_default(tmp_path, monkeypatch): diff --git a/tests/test_settings_tool_groups.py b/tests/test_settings_tool_groups.py index 693627ec3..9f209fe60 100644 --- a/tests/test_settings_tool_groups.py +++ b/tests/test_settings_tool_groups.py @@ -65,6 +65,9 @@ def test_tool_groups_cover_the_default_tools() -> None: search_api_key="test-serper-key", jina_api_key="test-jina-key", image_search=True, + # Opt-in like image_search: without the flag the loop cannot + # register the tool, and an ungrouped one would pass unseen. + connection_add=True, tool_search_config=ToolSearchConfig(enabled=True), ), engine=EngineWiring(), diff --git a/tests/test_shell_machine_channel.py b/tests/test_shell_machine_channel.py index 868d52a92..49a86276a 100644 --- a/tests/test_shell_machine_channel.py +++ b/tests/test_shell_machine_channel.py @@ -740,11 +740,22 @@ def test_the_table_agrees_with_the_ssh_on_this_computer(command, expected): if not shutil.which("ssh"): pytest.skip("no ssh on this computer") words = command.split()[1:] - out = subprocess.run(["ssh", "-G", *words], capture_output=True, text=True, check=False, timeout=10).stdout - seen = dict(line.split(" ", 1) for line in out.splitlines() if line.startswith(("hostname ", "port "))) - if "-P" in words and "tag" not in out: + + def resolved(args: list[str]) -> tuple[str, int]: + out = subprocess.run(["ssh", "-G", *args], capture_output=True, text=True, check=False, timeout=10).stdout + seen = dict(line.split(" ", 1) for line in out.splitlines() if line.startswith(("hostname ", "port "))) + return seen.get("hostname", ""), int(seen.get("port", 0) or 0) + + if "tag" in words and resolved(["-P", "tag", "host.invalid"])[0] != "host.invalid": + # ``-P`` takes no value before OpenSSH 9.2, where ``tag`` would BE the + # host: this row's expectation is the newer client's reading, so it is + # skipped rather than failed. Asked as a question rather than read off a + # version string, and asked as its own invocation so it cannot perturb + # the row's own arguments (reviewed 2026-09-24: the earlier check looked + # for the literal "tag" in the output, which an older ssh prints as the + # hostname -- so it never skipped). pytest.skip("this ssh predates `-P tag` (OpenSSH < 9.2), where P takes no value") - assert (seen.get("hostname"), int(seen.get("port", 0))) == expected + assert resolved(words) == expected def test_the_value_taking_flags_are_ssh_s_own(): @@ -767,3 +778,33 @@ async def test_a_port_written_after_the_host_is_still_refused(registry, tmp_path out = await tool.execute(command=command) assert "Error:" in out and "conn_gpu" in out, command assert not marker.exists() + + +def test_the_machine_lane_appears_once_a_first_machine_is_added(tmp_path, monkeypatch): + """The reviewed shape (2026-09-24): a process that starts with no machine, + then gets its first one from ``ops_connection_add``. The registry used to + serve exec from the copy it took at registration, so ``machine`` stayed + missing until a restart -- while every text after the add sent the model + to ``exec(machine=...)``. Written through the real writer and read back + through the real registry, not a patched ``load``.""" + from raven.agent.tools.registry import ToolRegistry + from raven.ops import connection_add, connections + + store = tmp_path / "connections.json" + monkeypatch.setattr(connections, "store_path", lambda: store) + registry = ToolRegistry() + registry.register(ExecTool(working_dir=str(tmp_path))) + + def served() -> dict: + (exec_def,) = [d for d in registry.get_definitions() if d["function"]["name"] == "exec"] + return exec_def["function"] + + before = served() + assert "machine" not in before["parameters"]["properties"] + assert "machine" not in before["description"] + + connection_add.write(dict(ROW, transport="ssh")) + + after = served() + assert "machine" in after["parameters"]["properties"], "the lane the reply names is in the served schema" + assert "pass 'machine'" in after["description"] diff --git a/ui-web/src/features/settings/toolGroups.json b/ui-web/src/features/settings/toolGroups.json index d34fc38a5..d310dc9a2 100644 --- a/ui-web/src/features/settings/toolGroups.json +++ b/ui-web/src/features/settings/toolGroups.json @@ -9,7 +9,8 @@ "deliver_files" ], "run": [ - "exec" + "exec", + "ops_connection_add" ], "net": [ "web_search", From 5146f0a433c63c06fba61eb72df17bb705b3044f Mon Sep 17 00:00:00 2001 From: "xiaotian.luo" Date: Thu, 24 Sep 2026 17:11:01 +0800 Subject: [PATCH 4/6] fix(*): format the registry resolver and count the new tool's settings row The settings Tools page now draws one more switch row (ops_connection_add joined the run group), so the skeleton test expects 23, and ruff format wraps the store_path docstring. Co-authored-by: Claude (claude-opus-5-5) --- raven/ops/connections.py | 1 + ui-web/src/features/settings/Skeletons.test.tsx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/raven/ops/connections.py b/raven/ops/connections.py index 942d97b97..ce543b72d 100644 --- a/raven/ops/connections.py +++ b/raven/ops/connections.py @@ -323,6 +323,7 @@ def store_path() -> Path: return home return beside if beside.is_file() else home + MISSING, UNREADABLE, OK = "missing", "unreadable", "ok" diff --git a/ui-web/src/features/settings/Skeletons.test.tsx b/ui-web/src/features/settings/Skeletons.test.tsx index 311580998..7247935c3 100644 --- a/ui-web/src/features/settings/Skeletons.test.tsx +++ b/ui-web/src/features/settings/Skeletons.test.tsx @@ -103,7 +103,7 @@ describe('the wait a settings section draws', () => { it('waits on the tool and plugin pages as their switch rows', async () => { await waiting('tools') - expect(count('.settings-xrow')).toBe(22) + expect(count('.settings-xrow')).toBe(23) cleanup() store._resetForTests() await waiting('plugins') From d7c5220e94f778e2f7dbcd67336110357970f014 Mon Sep 17 00:00:00 2001 From: "xiaotian.luo" Date: Thu, 24 Sep 2026 17:46:37 +0800 Subject: [PATCH 5/6] fix(ops): pick the registry by what the process is, not by which file exists Review question on #690 (2026-09-24): beside-the-config-first could not tell a stale copy beside a sub-agent's rendered config from the owner's list beside a `--config` host's config, and it broke the first the way home-first broke the second -- a copy read first hid every row added to the home since, and hid the coding agent's own adds from the on-call agent. `store_path()` now keys on the process role: a sub-agent (the host launched it with `RAVEN_SUBAGENT`) reads the owner's home whenever the home holds a registry, and its copy only when it does not (an install older than the home registry); any other instance, the `--config` host included, reads the list beside its config first, then the home. The on-call launcher's `connections_registry()` goes back to home-first for the same reason. Tests: the sub-agent fixture now carries `RAVEN_SUBAGENT`; the `--config` case runs as a host; two new cases pin both reproductions (a stale copy beside a sub-agent's config loses to the home, and one sub-agent's add reaches another whose state directory holds a copy). Both fail with the role check removed. Co-authored-by: Claude (claude-opus-5-5) --- CHANGELOG.md | 8 ++-- CONTEXT.md | 5 ++- agents/raven-oncall/run.py | 18 +++++--- docs-site/docs/oncall.md | 9 ++-- docs-site/docs/oncall.zh.md | 6 +-- raven/ops/connections.py | 45 +++++++++++-------- tests/test_agents_oncall_launcher.py | 10 ++--- tests/test_ops_connections.py | 64 +++++++++++++++++++++++++--- 8 files changed, 118 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 941e4643b..6b34ca862 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,10 +34,10 @@ All notable changes to Raven are documented here. ### Changed -- The connection registry falls back to the owner's home - (`raven_home()/connections.json`) when there is none beside the instance's - own config, and a first write lands there. A list beside the config still - comes first, so a `--config` host keeps its own file. A sub-agent runs on a rendered +- A sub-agent reads the connection registry in the owner's home + (`raven_home()/connections.json`), which the host hands it as `RAVEN_HOME`; + any other instance reads the one beside its config first, so a `--config` + host keeps its own file. A first write lands in the home. A sub-agent runs on a rendered config in a state directory of its own and is handed `RAVEN_HOME`, not a copy of the file; resolved beside the config alone, Raven-Code read an empty directory while the owner's machines sat one level up, and reached diff --git a/CONTEXT.md b/CONTEXT.md index 37a226356..e85fae238 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -340,8 +340,9 @@ no subprocess and no launcher. A compute host the owner registered — with `raven ops connection add`, or, since 2026-09-23, with the `ops_connection_add` tool an agent drives from the owner's answers in conversation. The registry is the path `RAVEN_CONNECTIONS` -points at, else `connections.json` beside the active config, else the one in -the raven home, where a first registration lands. An on-call-style agent runs +points at; else, for a sub-agent, the one in the raven home the host hands it, +and for any other instance the one beside its active config, falling back to +the home, where a first registration lands. An on-call-style agent runs its work outside the dispatching Raven process — on a GPU box, a lab workstation, another machine entirely — and the host's whole part in that is keeping the registry and handing it over: a launcher points the agent's diff --git a/agents/raven-oncall/run.py b/agents/raven-oncall/run.py index a576549e1..e828c1470 100644 --- a/agents/raven-oncall/run.py +++ b/agents/raven-oncall/run.py @@ -116,14 +116,20 @@ def connections_registry() -> Path: rendered config -- which is the state root here -- so the on-call tools would see no machines at all, and a task naming a remote path would be read as naming a local one. A pointer rather than a copy: a copy is wrong - from the first machine the owner adds, with nothing to say so. An install - that predates this and keeps its own list stays on it -- checked first, so a - home registry appearing later (an agent's first add writes one) does not - quietly take its place (reviewed 2026-09-24: home-first dropped such a list - at the next launch, both rows still on disk and unread). + from the first machine the owner adds, with nothing to say so. + + The owner's home when it holds a registry; a list of this install's own + only when the home has none. The home comes first because a list in the + state root is a copy -- nothing writes there since 2026-09-01 -- and read + first it hid every machine added to the home since, including the coding + agent's own adds, which land in the home (reviewed 2026-09-24). An install + that predates the home registry and kept its own list still reads it. """ + host = raven_home() / "connections.json" + if host.is_file(): + return host own = state_root() / "connections.json" - return own if own.is_file() else raven_home() / "connections.json" + return own if own.is_file() else host def seed_guide(workspace: Path) -> None: diff --git a/docs-site/docs/oncall.md b/docs-site/docs/oncall.md index 30f8510dd..eae2b15ed 100644 --- a/docs-site/docs/oncall.md +++ b/docs-site/docs/oncall.md @@ -50,10 +50,11 @@ raven ops connection add --id local-lab --name "Local lab" \ --non-interactive ``` -The registry is the path selected by `RAVEN_CONNECTIONS`; else the -`connections.json` beside the active config, when there is one; else the one in -the raven home (`RAVEN_HOME`, or `~/.raven`), which is also where a first -registration lands. Sub-agents run on a +The registry is the path selected by `RAVEN_CONNECTIONS`. Otherwise a +sub-agent reads the one in the raven home (`RAVEN_HOME`, or `~/.raven`) that the +host hands it, and any other instance -- the host itself, including one started +with `--config` -- reads the `connections.json` beside its config when there is +one, else the home. A first registration lands in the home. Sub-agents run on a rendered config in a state directory of their own and inherit the home, so they read the owner's registry rather than a copy of it. SSH registration can also write a managed alias in `~/.ssh/config`; the key path is recorded, not diff --git a/docs-site/docs/oncall.zh.md b/docs-site/docs/oncall.zh.md index bd4d9e0f7..2a5b40d1d 100644 --- a/docs-site/docs/oncall.zh.md +++ b/docs-site/docs/oncall.zh.md @@ -41,9 +41,9 @@ raven ops connection add --id local-lab --name "Local lab" \ --non-interactive ``` -注册表的位置:有 `RAVEN_CONNECTIONS` 就用它指的文件;否则当前 config 旁边有 -`connections.json` 就用那份;都没有时用 raven 家目录(`RAVEN_HOME`,默认 `~/.raven`)下的那份, -第一次注册也写在这里。 +注册表的位置:有 `RAVEN_CONNECTIONS` 就用它指的文件。否则,子代理读宿主交给它的 raven 家目录 +(`RAVEN_HOME`,默认 `~/.raven`)下的那份;其他实例(宿主本身,包括用 `--config` 启动的) +先读 config 旁边的 `connections.json`,没有再读家目录。第一次注册写在家目录。 子代理各自用渲染出的 config 跑在自己的状态目录里、继承家目录，所以读到的是 主人的注册表，不是一份副本。SSH 注册还可能在 `~/.ssh/config` 写入受管理的别名； 只记录私钥路径，不记录私钥内容。不要把密钥或密码放进任务消息。 diff --git a/raven/ops/connections.py b/raven/ops/connections.py index ce543b72d..e80926b25 100644 --- a/raven/ops/connections.py +++ b/raven/ops/connections.py @@ -292,31 +292,42 @@ def usable(rows: list[dict[str, Any]]) -> list[dict[str, Any]]: def store_path() -> Path: """Where this instance reads its machines, and where a first add lands. - The env var when set; else the registry beside this instance's own config - when one is there; else the owner's home (``raven_home()``), which is also - where a machine added now belongs. - - The home is the fallback rather than a plain "beside the config" because a - sub-agent runs on a rendered config in a state directory of its own, where - nothing is ever written, and the host hands it ``RAVEN_HOME`` -- not a copy - of the file. Resolved beside the config alone, the coding agent read that - empty directory while the owner's machines sat in the home (measured - 2026-09-22: ``exec(machine=...)`` answered "No connection is registered" on - a home that listed two), and the model reached for the raw address instead. - - A list beside the config comes first so that one never moves: a host started - with ``--config /x/config.json`` beside its own ``/x/connections.json`` - keeps reading that file after a registry first appears in the home -- which - an agent's first add now writes without the owner doing anything (reviewed - 2026-09-24: home-first switched such a host to the new file, silently). + The env var when set. Otherwise the answer depends on what this process is, + not on which file happens to exist: + + * **a sub-agent** (the host launched it with ``RAVEN_SUBAGENT``) reads the + owner's home (``raven_home()``), which the host hands it as + ``RAVEN_HOME``, whenever the home holds a registry. Its config is a + rendered copy in a state directory of its own, and a registry beside it + can only be a copy taken to keep it supplied -- the copy-once bug (five + byte-identical registries on one computer, 2026-08-25). Read first, such + a copy hid every machine the owner added since, and hid the coding + agent's own adds from the on-call agent; read never, the coding agent saw + nothing at all (measured 2026-09-22: ``exec(machine=...)`` answered "No + connection is registered" on a home that listed two, and the model + reached for the raw address). The copy is read only when the home has + none, which is an install older than the home registry. + * **anything else** -- the host itself, including one started with + ``--config /x/config.json`` -- reads the registry beside that config when + one is there, else the home. The list beside a config the owner chose is + the owner's own, and must not be swapped out when a home registry first + appears, which an agent's first add now writes with no owner action. + + Keyed on the process role rather than on file presence (reviewed + 2026-09-24): a stale copy beside a rendered config and an owner's list + beside a ``--config`` file look identical on disk, so each file-presence + order broke one of them. """ override = os.environ.get(CONNECTIONS_ENV, "").strip() if override: return Path(override).expanduser() + from raven.agent.subagent.role import is_subagent_process from raven.config.paths import get_config_path from raven.home import raven_home home = raven_home() / STORE + if is_subagent_process() and home.is_file(): + return home try: beside = Path(get_config_path()).expanduser().parent / STORE except Exception: # noqa: BLE001 -- a missing config path is not a failure here diff --git a/tests/test_agents_oncall_launcher.py b/tests/test_agents_oncall_launcher.py index cb660a6f9..0d812a779 100644 --- a/tests/test_agents_oncall_launcher.py +++ b/tests/test_agents_oncall_launcher.py @@ -259,17 +259,17 @@ def test_a_preexisting_own_registry_stays(grounded, tmp_path): assert grounded.connections_registry() == own -def test_an_own_registry_stays_after_a_home_registry_appears(grounded, tmp_path): - """An agent's first add writes a home registry with no owner action; an - install that kept its own list must not drop it at the next launch - (reviewed 2026-09-24).""" +def test_the_owners_home_wins_over_an_own_copy(grounded, tmp_path): + """A list in the state root is a copy (nothing writes there since + 2026-09-01). Read first, it hid every machine added to the home since, + including the coding agent's adds (reviewed 2026-09-24).""" own = tmp_path / "state" / "connections.json" own.parent.mkdir(parents=True, exist_ok=True) own.write_text("[]") home = tmp_path / "home" home.mkdir(parents=True, exist_ok=True) (home / "connections.json").write_text("[]") - assert grounded.connections_registry() == own + assert grounded.connections_registry() == home / "connections.json" # --- the visible tool face ---------------------------------------------------- diff --git a/tests/test_ops_connections.py b/tests/test_ops_connections.py index e75a2cc41..b22df122b 100644 --- a/tests/test_ops_connections.py +++ b/tests/test_ops_connections.py @@ -385,6 +385,8 @@ def _rendered_config(tmp_path, monkeypatch): monkeypatch.setattr(home, "_current_config_path", rendered) monkeypatch.setenv("RAVEN_HOME", str(tmp_path / "home")) monkeypatch.delenv(connections.CONNECTIONS_ENV, raising=False) + # What the host overlays on every child it launches (``subagent_role_env``). + monkeypatch.setenv("RAVEN_SUBAGENT", "1") (tmp_path / "home").mkdir() return state @@ -426,13 +428,13 @@ def test_an_install_that_kept_its_own_list_beside_its_config_stays_on_it(tmp_pat assert [r["id"] for r in connections.load()] == ["mine"] -def test_a_list_beside_the_config_keeps_winning_after_a_home_registry_appears(tmp_path, monkeypatch): - """The reviewed shape (2026-09-24): a host started with ``--config`` beside - its own list. An agent's first add now writes a home registry with no - owner action, and home-first switched that host to the new file -- its own - machines still on disk, no longer read. The list beside the config stays - the answer.""" +def test_a_config_host_keeps_its_own_list_after_a_home_registry_appears(tmp_path, monkeypatch): + """A host started with ``--config`` beside its own list -- not a sub-agent. + An agent's first add now writes a home registry with no owner action, and + a home-first rule switched that host to the new file, its own machines + still on disk and no longer read (reviewed 2026-09-24).""" state = _rendered_config(tmp_path, monkeypatch) + monkeypatch.delenv("RAVEN_SUBAGENT") own = state / "connections.json" own.write_text(json.dumps({"connections": [{"id": "conn_cpu_32c", "display_name": "CPU", "transport": "local"}]})) assert connections.store_path() == own @@ -445,6 +447,56 @@ def test_a_list_beside_the_config_keeps_winning_after_a_home_registry_appears(tm assert [r["id"] for r in connections.load()] == ["conn_cpu_32c"] +def test_a_sub_agent_reads_the_owners_home_over_a_stale_copy_beside_its_config(tmp_path, monkeypatch): + """The other half of the same review: beside-the-config-first read a copy + left in a sub-agent's state directory and lost every row the owner added + to the home since -- the copy-once bug. Six such copies existed on one + computer on 2026-09-24. A sub-agent reads the home whenever it has one.""" + state = _rendered_config(tmp_path, monkeypatch) + (state / "connections.json").write_text( + json.dumps({"connections": [{"id": "conn_cpu_32c", "display_name": "CPU", "transport": "local"}]}) + ) + theirs = tmp_path / "home" / "connections.json" + theirs.write_text( + json.dumps( + { + "connections": [ + {"id": "conn_cpu_32c", "display_name": "CPU", "transport": "local"}, + {"id": "conn_added_since", "display_name": "Added since", "transport": "local"}, + ] + } + ) + ) + + assert connections.store_path() == theirs + assert [r["id"] for r in connections.load()] == ["conn_cpu_32c", "conn_added_since"] + + +def test_what_one_sub_agent_adds_another_reads(tmp_path, monkeypatch): + """The coding agent's add must reach the on-call agent even where a stale + copy sits beside the on-call agent's config (reviewed 2026-09-24: with the + copy read first, on-call listed the old rows and never the new machine). + Two processes, two state directories: the writer's is empty, the reader's + holds the copy.""" + import raven.home as home_mod + from raven.ops import connection_add + + _rendered_config(tmp_path, monkeypatch) + row = {"id": "conn_cpu_32c", "display_name": "CPU", "transport": "local"} + (tmp_path / "home" / "connections.json").write_text(json.dumps({"connections": [row]})) + code_state, oncall_state = tmp_path / "code", tmp_path / "oncall" + for d in (code_state, oncall_state): + d.mkdir() + (d / ".config.rendered.json").write_text("{}") + (oncall_state / "connections.json").write_text(json.dumps({"connections": [row]})) + + monkeypatch.setattr(home_mod, "_current_config_path", code_state / ".config.rendered.json") + connection_add.write({"id": "conn_new", "display_name": "New", "transport": "local"}) + + monkeypatch.setattr(home_mod, "_current_config_path", oncall_state / ".config.rendered.json") + assert [r["id"] for r in connections.load()] == ["conn_cpu_32c", "conn_new"] + + def test_the_env_var_still_outranks_every_default(tmp_path, monkeypatch): _rendered_config(tmp_path, monkeypatch) (tmp_path / "home" / "connections.json").write_text(json.dumps({"connections": []})) From dbfc784fcfa774f216a716c45608610588a1aa56 Mon Sep 17 00:00:00 2001 From: "xiaotian.luo" Date: Thu, 24 Sep 2026 18:02:46 +0800 Subject: [PATCH 6/6] refactor(ops): read the sub-agent role from raven.home store_path() imported raven.agent.subagent.role, which made raven.ops and raven.agent import each other and pushed the mutual-pair count to 14, over the ceiling of 13. The role variable, its reader and its writer now live in raven.home, which every shelf may import; raven.agent.subagent.role re-exports them so existing callers are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) --- raven/agent/subagent/role.py | 27 +++++---------------------- raven/home.py | 36 +++++++++++++++++++++++++++++++++++- raven/ops/connections.py | 3 +-- 3 files changed, 41 insertions(+), 25 deletions(-) diff --git a/raven/agent/subagent/role.py b/raven/agent/subagent/role.py index 1d01efdec..21f34e999 100644 --- a/raven/agent/subagent/role.py +++ b/raven/agent/subagent/role.py @@ -28,12 +28,11 @@ from __future__ import annotations -import os - -#: Set by the host on a child it launches to act as a sub-agent; read only for -#: truth, so a config ``env`` entry of "0" is the way back to a full registry -#: (that map is merged after the host's, and so wins). -SUBAGENT_ENV_VAR = "RAVEN_SUBAGENT" +# The variable, its reader and its writer live in ``raven.home``, the one module +# every shelf may import: ``raven.ops`` reads the role to pick whose machine list +# is authoritative, and ops importing raven.agent would tie the two packages into +# a cycle. Re-exported here so the sub-agent code keeps its own address for them. +from raven.home import SUBAGENT_ENV_VAR, is_subagent_process, subagent_role_env #: Every tool that dispatches work to another agent, or steers a dispatch already #: running. Withheld from a sub-agent because a node's whole point is that *this* @@ -77,20 +76,4 @@ ) -def is_subagent_process() -> bool: - """Whether the host launched this process to serve as a sub-agent.""" - return os.environ.get(SUBAGENT_ENV_VAR, "").strip().lower() in ("1", "true", "yes", "on") - - -def subagent_role_env() -> dict[str, str]: - """What the host overlays on a child it launches to answer as a sub-agent. - - Written next to the reader above rather than beside the other environment - builders: the two are one contract, and a variable named in two files is one - rename away from a child that is told nothing and silently keeps the full - registry -- the failure this exists to close, and one no error reports. - """ - return {SUBAGENT_ENV_VAR: "1"} - - __all__ = ["SUBAGENT_ENV_VAR", "WITHHELD_FROM_SUBAGENT", "is_subagent_process", "subagent_role_env"] diff --git a/raven/home.py b/raven/home.py index 1dca09eeb..28c727446 100644 --- a/raven/home.py +++ b/raven/home.py @@ -11,6 +11,11 @@ ``raven.config.loader`` re-exports all three names, so the shelves and the entrances keep reading them from where they always have. + +The sub-agent role flag sits here for the same reason: it says whose home a +process answers for (``raven.ops.connections`` reads it to pick the owner's +machine list), so every shelf has to reach it, and this is the one module they +all may import. ``raven.agent.subagent.role`` re-exports it. """ from __future__ import annotations @@ -20,7 +25,14 @@ from raven.contracts.path_policy import CONFIG_FILENAME, DEFAULT_HOME_DIRNAME, HOME_ENV_VAR -__all__ = ["get_config_path", "raven_home", "set_config_path"] +__all__ = [ + "SUBAGENT_ENV_VAR", + "get_config_path", + "is_subagent_process", + "raven_home", + "set_config_path", + "subagent_role_env", +] #: Set by a caller that runs against a config file of its own (tests, and a #: second instance). Wins over the environment and the default. @@ -49,3 +61,25 @@ def get_config_path() -> Path: if _current_config_path: return _current_config_path return raven_home() / CONFIG_FILENAME + + +#: Set by the host on a child it launches to act as a sub-agent; read only for +#: truth, so a config ``env`` entry of "0" is the way back to a full registry +#: (that map is merged after the host's, and so wins). +SUBAGENT_ENV_VAR = "RAVEN_SUBAGENT" + + +def is_subagent_process() -> bool: + """Whether the host launched this process to serve as a sub-agent.""" + return os.environ.get(SUBAGENT_ENV_VAR, "").strip().lower() in ("1", "true", "yes", "on") + + +def subagent_role_env() -> dict[str, str]: + """What the host overlays on a child it launches to answer as a sub-agent. + + Written next to the reader above rather than beside the other environment + builders: the two are one contract, and a variable named in two files is one + rename away from a child that is told nothing and silently keeps the full + registry -- the failure this exists to close, and one no error reports. + """ + return {SUBAGENT_ENV_VAR: "1"} diff --git a/raven/ops/connections.py b/raven/ops/connections.py index e80926b25..6d1079b79 100644 --- a/raven/ops/connections.py +++ b/raven/ops/connections.py @@ -321,9 +321,8 @@ def store_path() -> Path: override = os.environ.get(CONNECTIONS_ENV, "").strip() if override: return Path(override).expanduser() - from raven.agent.subagent.role import is_subagent_process from raven.config.paths import get_config_path - from raven.home import raven_home + from raven.home import is_subagent_process, raven_home home = raven_home() / STORE if is_subagent_process() and home.is_file():