diff --git a/.gitignore b/.gitignore index 3d850f6d0..d7c3b3fd0 100644 --- a/.gitignore +++ b/.gitignore @@ -62,3 +62,11 @@ mission.md .spur_job_*.sh .spur_ns_*.sh spur-*.out + +# serena writes `.serena/` into whatever directory it is given as `--project`, +# and `examples/env_checker`'s capability-7 measurements point it at the agent +# asset directory. Config and symbol cache, regenerated on demand, belonging to +# no package. Ignored rather than remembered: `launch.sh` refuses to start a run +# while the package tree is dirty, so an untracked by-product that everybody +# knows to skip would block the gate and teach people to bypass it. +.serena/ diff --git a/agent_sys/agent/backend.py b/agent_sys/agent/backend.py index 9765b9cd3..370d7b80e 100644 --- a/agent_sys/agent/backend.py +++ b/agent_sys/agent/backend.py @@ -182,6 +182,21 @@ class Assignment(BaseModel): #: which is what every backend but `claude_sdk` does today. tools: tuple[Any, ...] = () + #: **External MCP servers this agent's components declared**, keyed by the + #: name the model addresses them under. `env_mgr.Prepared.mcp_servers`, + #: straight through. + #: + #: Typed loosely for `tools`' reason and one more of its own: the values are + #: the *SDK's* server vocabulary — `{"type": "stdio", "command": …}` and the + #: rest — so a type here would be `agent` declaring a shape it does not own + #: and cannot check, on behalf of one of its backends. + #: + #: **A field and not prose in the readme**, for the reason spec §5.5 gives + #: `tools`: an agent told in English to start an MCP server will improvise. + #: A backend that cannot express external servers ignores this entirely, + #: which is every backend but `claude_sdk` today. + mcp_servers: dict[str, Any] = Field(default_factory=dict) + #: `env_mgr.Confinement`, carried for an executor that wants to report what #: it will run under. Typed loosely because `agent` may not import `env_mgr`. #: diff --git a/agent_sys/agent/backends/claude_sdk.py b/agent_sys/agent/backends/claude_sdk.py index 3c4e8663e..35d40e9f8 100644 --- a/agent_sys/agent/backends/claude_sdk.py +++ b/agent_sys/agent/backends/claude_sdk.py @@ -357,6 +357,34 @@ def _options(self) -> dict[str, Any]: options.setdefault("permission_mode", "bypassPermissions") if self.assignment.environment: options.setdefault("env", dict(self.assignment.environment)) + if self.assignment.mcp_servers: + # **The per-agent components' external servers, under the tool + # server's own collision policy.** Not a second policy: the reason a + # name may not be taken twice is that `mcp____` is what + # the model calls, and that is true of a component's server exactly + # as it is of `env_mgr`'s. So the refusal below is the same refusal, + # said about a different name. + # + # **Refused rather than merged, and the direction matters.** The + # caller's `options["mcp_servers"]` is an operator's configuration; + # `assignment.mcp_servers` is what a package declared. Letting either + # win silently means one of the two gets different tools than the + # ones they wrote, with nothing said — the defect the `env_mgr` key + # already carries a comment about, one collision wider. + servers = dict(options.get("mcp_servers") or {}) + clash = sorted(set(servers) & set(self.assignment.mcp_servers)) + if clash: + raise BackendUnsupported( + self.key, + "mcp_servers", + f"this config and this agent's components both declare MCP " + f"server(s) {clash}. The model addresses these as " + f"mcp____, so two servers cannot share a name — " + f"rename one side rather than letting the other's tools " + f"disappear.", + ) + servers.update(self.assignment.mcp_servers) + options["mcp_servers"] = servers if self.assignment.tools: # **Spec §5.5's remote surface, and the only place that knows the # SDK.** `env_mgr` may not import the SDK and `agent/backend.py` is diff --git a/agent_sys/agent/docs/design.md b/agent_sys/agent/docs/design.md index b3892334a..ff452c6b1 100644 --- a/agent_sys/agent/docs/design.md +++ b/agent_sys/agent/docs/design.md @@ -1297,7 +1297,7 @@ test the chain against nothing that can actually fail to be available. | **O1** | **Criterion 13 is not testable as written.** "Losslessly for what both support" requires knowing the intersection of two harnesses' feature sets, and no converter computes it — everyone hand-maintains a table. Both reference implementations' tables fail invisibly: **pandoc** classifies a dropped block as `INFO`, so `--fail-if-warnings` exits 0 with the content gone, and attribute-level loss (a link title) is logged at *no* level; **kompose**'s 25-entry unsupported-key table has **no production caller** — its only caller is its own unit test, and the exported sibling is invoked with an empty map, so eight declared-unsupported keys converted clean with exit 0. The only executable formulation found anywhere is `rulesync`'s per-(target, feature) fixtures asserting the *canonical* value. **The criterion needs to name the artefact that defines "what both support" and the test that keeps it honest** — and to separate *unsupported* (the target cannot express it) from *unknown* (the converter did not handle it), which GitHub Actions Importer does and which criterion 13 conflates | | **O2** | **Which `claude` CLI the backend runs is undecided, and it is `env_mgr`'s call.** The SDK prefers its bundled 328 MB executable; `env_mgr` installs plugins into whatever is on `PATH`. Unless `cli_path` is set from the prepared environment, an agent does not see the plugins its own recipe installed. §8.7 | | **O3** | **The backend's transcript lands outside the confinement zone.** `~/.claude/projects//*.jsonl`, containing prompts and reasoning, written by default. Criterion 16 is about the system's record and stays true, but "an agent reaches only its own zone" does not. Three levers exist (`CLAUDE_CONFIG_DIR`, `CLAUDE_CODE_SKIP_PROMPT_HISTORY`, a `SessionStore`); choosing one is `env_mgr`'s, and its spec does not mention the directory | -| **O4** | **§4.5's "Claude Code's format" is ambiguous.** The declarative `.claude/settings.json` surface and the SDK's `ClaudeAgentOptions(hooks={...})` callbacks are different execution models. Every surveyed converter targets the first; **nobody converts programmatic callbacks at all.** §4.5 must name which surface is canonical, and if it is the callbacks, criterion 13 has no prior art of any kind behind it | +| **O4** | **ANSWERED — the declarative `settings.json` surface is canonical.** The question was whether *"Claude Code's format"* meant the `.claude/settings.json` tree or the SDK's `ClaudeAgentOptions(hooks={...})` callbacks; every surveyed converter targets the first and **nobody converts programmatic callbacks at all.** Per-agent components forced the ruling, because they had to be *stored* somewhere: L2 and L3 are `.claude/` trees, `env_mgr/agent_assets.py` writes `/config/settings.json` and points `CLAUDE_CONFIG_DIR` at it, and `spec.md` §4.5 now says so. The warned-of consequence is therefore not incurred — criterion 13 rests on the surface the prior art covers. The callback form stays legal in a backend `config` and is passed through; it is simply not the stored form. **What is still open is narrower and belongs to O1**: no converter exists, so "canonical" is currently a claim about one harness rather than a demonstrated N-to-1 | | **O5** | **Two objects hold "the agent spec table".** `AgentSpecRegistry` here, and `AgentMgr.register(spec, **config)` in `task_graph`, which copies its dict onto every minted `Agent.config`. This design assumes the loader feeds the second from the first and that nothing else writes either — but the direction is not stated in any spec, and `engineer_principle.md` §1 forbids two writers for one fact | | **O6** | **How each phase becomes separately attributable** — narrowed twice, and the question is now smaller than rev. 3 stated it. `validator` design §8.2 rev. 2 owns the **requirement**: a phase must carry an `agent_id`, because criterion 10 there is untestable otherwise, and the SDK's `agent_id` is *"absent on the main thread"*. This module owns the **mechanism**, and one candidate is ruled out: not one client with several `session_id`s, because `interrupt()` takes no `session_id` and acts on the whole connection (§8.4). `fork_session`, `resume`, a subagent per phase, and a second client remain, and none was tested. The stage-three consistency pass found this document and `validator`'s giving different answers to what turned out to be two different questions; splitting them is what made the residue this small | | **O7** | **Mid-run backend failure.** §3.3's "pins the whole run" implies no fallback after the chosen backend dies, and every surveyed project except LiteLLM agrees. LiteLLM's cost is on record — a depth bound, an attempted-targets set against looping graphs, a pin predicate, cooldown feedback and per-failure-class chains, threaded through a loosely-typed `kwargs` at four call sites. Worth knowing before anyone proposes it, and worth stating in the spec either way | diff --git a/agent_sys/agent/docs/spec.md b/agent_sys/agent/docs/spec.md index 7e9cecc7e..f1333375d 100644 --- a/agent_sys/agent/docs/spec.md +++ b/agent_sys/agent/docs/spec.md @@ -104,6 +104,16 @@ spec** — the two are declared together in a closure. | `env` | Environment requirements, resolved by `env_mgr` | | `knowledge` | §3.4 | | `rules` / `hooks` / `skills` | Configuration, stored in canonical form. §4.5 | +| `assets` | **Filled by `spec_loader`, not written.** This agent's own directory under the package's `assets/`, found by the same three folder spellings a body lookup uses — `X`, `X.agent`, `agent.X`. Two matching directories is `SpecInconsistent`; an explicit binding is legal and warns. §4.5a | +| `recipes` | The **agent layer** of three recipe layers; `env_mgr` recipe YAMLs as `agent_sys:` or `package:` — a reference names its root. §4.5a | + +Nine keys became eleven, and both additions are one thing: **an agent may now +carry components, not only files.** §4.5a is why that needed new keys instead of +a longer `skills` list. + +**There was a third, `agent_plugins:`, and it is deleted** — +`docs/spec.provisioning.md` §4: what this repository ships under +`env_mgr/addons/` is installed by a recipe, and no declaration key reaches it. ### 3.2 Permissions are not here @@ -377,6 +387,62 @@ Picking one canonical format matters more than which one is picked: with N harnesses, storing each in its own format needs N² converters, and storing one canonical form needs N. +**Which Claude Code surface is canonical: the declarative one.** Design O4 asked, +because *"Claude Code's format"* named two different execution models — the +`.claude/settings.json` tree and the SDK's `ClaudeAgentOptions(hooks={...})` +callbacks — and every surveyed converter targets the first while nobody converts +programmatic callbacks at all. The answer is the first, and it is now what the +code does: `env_mgr` writes `/config/settings.json` and points +`CLAUDE_CONFIG_DIR` at it. The consequence O4 warned about is therefore not +incurred — criterion 13 rests on the surface the prior art actually covers. + +The callback form is not forbidden; a backend config may still carry `hooks`, and +`claude_sdk` passes it through. What it is not is the **stored** form, so nothing +in a package or a component is written that way. + +### 4.5a A component is a tree, and that needed three keys + +§4.5's three lists are lists of *files*. A Claude Code component is a directory: +a skill is a directory, a plugin marketplace is a directory of directories, and +an MCP server is a process to register rather than a file to place. Naming every +file would make a package author restate a layout the harness already fixes. + +**Two routes, and only one copies a tree.** `docs/spec.provisioning.md` is +normative here and supersedes both the L1/L2/L3 numbering and the three-origins +table that replaced it. + +| what | declared how | +|---|---| +| anything upstream ships, and anything `agent_sys` ships under `env_mgr/addons/` | `recipes: [...]`, or the package/default recipe layer; `tags: [internal]` marks an item as ours | +| what one task package carries for one agent | **undeclared** — `/.claude/`, copied | + +**The copied tree is in Claude Code's canonical layout**: +`settings.json`, `skills//`, `plugins/` (a local marketplace), +`.mcp.json`, and `tools/*.mcp.py`. It is the harness's own layout rather than +ours, so a file is placed and not converted. (A `tools/*.tooldef.py` was a fourth +member until 2026-09-04, when the in-process route it used was deleted — +`docs/spec.provisioning.md` §6.) + +**A package's own material is undeclared on purpose.** A declaration would be a +second statement of what the directory already says, and the two would drift the +first time somebody moved it without editing the YAML. + +`env_mgr/agent_assets.py` installs both; `env_mgr/docs/design.md` §11.5a is +the mechanism, including the measured ordering constraint that decides when +`settings.json` is written, the marketplace copy probe F forced, and why a +recipe runs the shipped machinery as a subprocess. What reaches this package +from a component is `Assignment.mcp_servers`. `Assignment.tools` also exists, +but nothing a component ships arrives through it — it carries `env_mgr`'s own +remote surface and nothing else (§5.5). + +**A component names a binary through `${VAR}`, never through `PATH`.** An +`.mcp.json` entry is expanded against the zone environment before it becomes an +`mcp_servers` entry, and an unresolved name is an error. That is not a +convenience: `PATH` is derived from the granted policy at prepare step 2, and a +directory a recipe installs into does not exist until step 6b — so +`"${UV_TOOL_BIN_DIR}/serena"` is the only spelling that works, and it is the one +measured working. + --- ## 5. `claude-agent-sdk` as the first backend diff --git a/agent_sys/agent/runner.py b/agent_sys/agent/runner.py index 2acd8d58a..087fec030 100644 --- a/agent_sys/agent/runner.py +++ b/agent_sys/agent/runner.py @@ -761,6 +761,10 @@ def _deploy(self, spec: Any) -> Executor: # `Prepared` from before this field existed is still a valid one, # which is the same allowance every other optional field here gets. tools=tuple(getattr(prepared, "tools", ()) or ()), + # The per-agent components' external servers, same allowance and + # for the same reason: a `Prepared` built before this field existed + # is still a valid one. + mcp_servers=dict(getattr(prepared, "mcp_servers", None) or {}), confinement=getattr(prepared, "confinement", None), agent_cli=prepared.agent_cli, # **Read, not inferred**, and that is the field's whole reason. diff --git a/agent_sys/agent/spec.py b/agent_sys/agent/spec.py index 4f353c3b8..fe99bc9ec 100644 --- a/agent_sys/agent/spec.py +++ b/agent_sys/agent/spec.py @@ -106,6 +106,34 @@ class AgentSpec(_Model): hooks: list[str] = Field(default_factory=list) skills: list[str] = Field(default_factory=list) + #: This agent's own directory under the package's `assets/`, package-relative, + #: or `""` when it has none. **Filled by `spec_loader`, not written**, from the + #: same folder convention that scopes a task's body lookup. + #: + #: Spec §3.1 listed nine keys and this is a tenth, so it is a spec change and + #: not a model detail: what an agent carries turned out not to fit in `rules` / + #: `hooks` / `skills`, which are three lists of *paths to individual files*. A + #: Claude Code component is a **tree** — a skill is a directory, a plugin + #: marketplace is a directory of directories — and naming each file would make + #: the package author restate a layout the harness already fixes. + assets: str = "" + + #: `env_mgr` recipe YAMLs run before the session, each written + #: ``:`` — ``agent_sys:`` for one this repository ships + #: under `env_mgr/recipes/`, ``package:`` for one this task package + #: carries. **A reference names its root**: there is no bare form and no + #: fallback, because until 2026-09-04 the root was decided by which candidate + #: happened to exist. The + #: route by which an agent asks for components this repository does not ship + #: and should not vendor (serena, marketplace plugins, apt/pip tools) — and, + #: with an item carrying ``tags: [internal]``, for one that it does. + #: + #: **Also filled by convention** when the agent carries its own recipe: + #: `assets/env_recipe..yaml` and every other permutation of those + #: tokens (`spec_loader/assets.py`, `fill_agent_env_recipe`). Declaring it + #: by hand is legal, warns, and wins whole. + recipes: list[str] = Field(default_factory=list) + @field_validator("backends", mode="before") @classmethod def _normalise(cls, value: Any) -> Any: diff --git a/agent_sys/cli/main.py b/agent_sys/cli/main.py index 0acd84a24..2d7cf18ff 100644 --- a/agent_sys/cli/main.py +++ b/agent_sys/cli/main.py @@ -21,6 +21,7 @@ import argparse import logging +import os import shutil import sys from collections.abc import Sequence @@ -49,6 +50,7 @@ from env_mgr.prepare import EnvManager, permissions_enforced from env_mgr.protocols import NoConfinement, PrepareRefused, UnresolvedGrant from env_mgr.remote.connection import sync_transport +from env_mgr.servers import REGISTRY_ENV_VAR, owned_servers from env_mgr.sync import check_delete_scope from monitor import ( NullUserSink, @@ -209,7 +211,7 @@ def main(argv: Sequence[str] | None = None) -> int: try: if args.verb == "show": return _show(args, stream) - return _run(args, stream) + return _run(args, stream, stack) except package.PackageNotFound as exc: return _fail(stream, PRECONDITION, str(exc)) except SpecInvalid as exc: @@ -259,12 +261,16 @@ def _show(args: argparse.Namespace, stream: Stream) -> int: # run -def _run(args: argparse.Namespace, stream: Stream) -> int: +def _run(args: argparse.Namespace, stream: Stream, stack: ExitStack) -> int: if args.clean: return _clean(args, stream) if args.dry_run: return _dry_run(args, stream) - return _real_run(args, stream) + # `stack` reaches only `_real_run`: it is what stops the servers a run + # started, and the other two verbs start none. `clean` removes a directory; + # `dry-run` dispatches nothing, which its own body asserts rather than + # assumes. + return _real_run(args, stream, stack) def _clean(args: argparse.Namespace, stream: Stream) -> int: @@ -326,7 +332,7 @@ def _layout(args: argparse.Namespace) -> Layout: return layout_for(root).create() -def _real_run(args: argparse.Namespace, stream: Stream) -> int: +def _real_run(args: argparse.Namespace, stream: Stream, stack: ExitStack) -> int: """Everything. Needs credentials, a sandbox, and a model. The order of the two preconditions is measured rather than aesthetic: the @@ -351,6 +357,23 @@ def _real_run(args: argparse.Namespace, stream: Stream) -> int: promises = expectations.for_package(package.locate(args.package)) layout = _layout(args) + # **The servers this run starts are stopped when this block unwinds.** + # `env_mgr` starts them, so `env_mgr` stops them: this is a call site, not a + # transfer of responsibility, and nothing below has to remember to clean up. + # + # The path is set on `os.environ` and not passed, because that is the only + # channel that reaches the installer: a recipe runs as a grandchild + # (`agent_assets._run_recipe` shells `python -m env_mgr`), and its + # environment is built from this process's. It is a **per-run** constant, so + # unlike `CLAUDE_CONFIG_DIR` -- which `agent_assets._child_env` refuses to + # set globally because it is per-attempt and the runner is threaded -- there + # is no value here for two threads to take from each other. + # + # What this does and does not promise is in `env_mgr/servers.py`: servers + # stop on normal and handled-error exit, and leak on SIGTERM and SIGKILL. + registry_file = layout.run / "servers.json" + os.environ[REGISTRY_ENV_VAR] = str(registry_file) + stack.enter_context(owned_servers(registry_file)) root = package.locate(args.package) # **Read once, at start-up, and it is the run's fact rather than a task's.** # `env_mgr.prepare.permissions_enforced()` is the single reader of the diff --git a/agent_sys/docs/ROADMAP.md b/agent_sys/docs/ROADMAP.md index b14ea406a..ed545deb7 100644 --- a/agent_sys/docs/ROADMAP.md +++ b/agent_sys/docs/ROADMAP.md @@ -212,6 +212,7 @@ how much of the verdict it sees — both alpha-dependent, neither measured. | **Agent env reuse** | A task, or a validation phase, reusing an existing agent environment directly or with light modification. **Careful:** this must not blur the system's isolation standard, which is the whole reason each validation gets a fresh environment | | **Movable handoff storage** | A handoff record should carry enough detail to be trackable *and* be movable between storage locations. The alpha does the first and not the second | | **Sync direction and conflict** | The weak local↔remote mapping is `rsync`, which has a direction. Which side wins when both changed is unspecified, and "the caller decides" will lose data eventually | +| **If `agent_sys` must ever serve MCP itself** — *the **component-supplied** in-process route was deleted 2026-09-04; `remote/tools.py` stays as a named exception, and this row is its closing condition* | The owner's sketch, in their words: **一个单独的线程，在初始化的时候load起来** — a separate thread, loaded at init — **with the declaration installed into Claude as a plugin.** What was deleted is different and is not what this describes: a component's `tools/*.tooldef.py` ran **that component's Python inside the supervisor process**, so a third party's code shared an address space with the process holding the run's credentials, and there was no boundary to configure. **Only the component-supplied half went.** `ToolDef` itself is still defined and still running, in `env_mgr/remote/tools.py`, which is the one thing `Prepared.tools` now carries. A thread is still in-process and would not fix that on its own; what makes the sketch tolerable is that the *declaration* goes through the plugin mechanism like every other capability, so there is one install route and no parallel one. **Why it is not needed now:** an add-on that wants to offer a tool ships a standalone MCP server, and both transports are already served — **stdio** is spawned by the harness from a declaration, and **port-based** is started by `env_mgr` via the `run_server` installer (`env_mgr/installers/run_server.py`). Nothing an add-on can express today requires `agent_sys` to be the server. Revisit only if something does. **How this touches the one route that was kept:** `spec.provisioning.md` §6 leaves `env_mgr/remote/tools.py` as a standing exception — `env_remote_run` / `_push` / `_pull` are injected into `ClaudeAgentOptions` as a live object with nothing written to disk, so no installer can carry them — and its **closing condition** is reproviding those three as a standalone server started by `run_server`. That is the same `run_server` this row argues already suffices, so the exception closes on the route above and **not** on the sketch: building the thread is not what retires it. Until somebody does that work, §6 has exactly one exception and a second added by analogy is the rule being ignored | ## 6.1 **P0 RISK — an AI task is not confined, and confinement is the anti-cheating property** diff --git a/agent_sys/docs/TODO.md b/agent_sys/docs/TODO.md index 8e7329d28..005fc512d 100644 --- a/agent_sys/docs/TODO.md +++ b/agent_sys/docs/TODO.md @@ -29,6 +29,12 @@ from where) are the four that block something real. | 4a | **A package's layout must separate a task's `bin` from the validators' `bin`** — *user-owned, and it is the precondition for staging* | **F19 reversed to staging** (`interfaces.md` §4.16), so a task gets a copy of what it needs rather than a grant on the package root. That only closes `env_mgr` criterion 13 if **a task's executable set can be named without dragging `validators/` along** — which is a package-layout guarantee, not something `env_mgr` can enforce. The user owns it. Until it holds, staging moves the leak rather than closing it | +| 4j | **A run killed by a signal leaks its servers, and nothing ever reaps the registry** — *recorded with `run_server`, 2026-09-04* | `env_mgr/servers.py` guarantees exactly *"stopped on normal and handled-error exit"* — whenever `owned_servers` unwinds. It does **not** unwind on `SIGTERM` (no handler is installed) or on `SIGKILL`. **What it costs to leave undone**: a crashed or timed-out run leaves a **listening process** and a registry file that no later run reads, so the port stays taken for as long as the machine is up, and the only thing that will ever notice is the *next* run's port check — which by then can only report a conflict, because the holder is a stranger to it. On a shared host the leak is somebody else's problem before it is ours. **Two separable pieces, and the first is the cheap one.** (1) A **sweep at start-up**: read registries left under earlier run roots and stop anything whose `starttime` still matches, which needs no new mechanism and reuses `stop_all` unchanged. (2) Closing the `SIGKILL` case itself, which the sweep does not do. `prctl(PR_SET_PDEATHSIG, SIGTERM)` was **measured to work** on this host — with it a child died when its parent was `kill -9`'d, without it the child survived — but it is the wrong tool at the site that spawns these: the spawning process is the recipe child, which exits within seconds, so the server would die the moment its own install finished. It would need the supervisor to be the direct parent, which is an architecture change and not a flag. **Deliberately not built with `run_server`**: a sweep that reaps the wrong thing is worse than a leak, and deciding which run roots it may reach is an owner's call about scope, not an implementation detail | + +| 4k | **Installs run unconfined, and `env_mgr` §4 does not say so** — *measured 2026-09-04, recorded only; nothing changed and nothing should be* | `env_mgr`'s design makes confinement the load-bearing property, and **installs are an exception to it.** Measured while siting the server registry: `agent_assets.py::_run_cmd` is `subprocess.run(list(argv), capture_output=True, text=True, env=dict(environ), timeout=timeout)` — **no `preexec_fn`, no Landlock ruleset, and the full inherited environment** — so `python -m env_mgr bootstrap `, and every `run:` string an installer shells from it, executes with the supervisor's own reach. Nothing about that is accidental or obviously wrong: **an install writes outside every zone by definition**, which is what installing is, and confining it to a zone would defeat the purpose rather than harden it. The gap is documentary. §4 reads as though confinement is universal within `env_mgr`, and the one path that is deliberately outside it is named nowhere, so the next reader meets the exception by discovering it in `_run_cmd` rather than by being told. **What would close it: a sentence in §4 naming the install path as out of scope for confinement, and why.** Not a code change — recorded here rather than acted on, because the behaviour is pre-existing, arguably required, and this round is not the place to relitigate it | + +| 4l | **One fact, two readers, two different fields — and the disagreement was silent** — *instance fixed, class unrecorded until now, 2026-09-04* | *"Does this zone have a far side, and where?"* was answered in two places from two fields. `prepare.py:645 _remote_tools` read **`far_roots`**; the `paths.zone_env` call site forty lines above read **`ctx.mapping`**, which is **weak-only** because it is `sync`'s input and strength answers *must bytes be copied*. A **strong** mapping still has a far side and its `remote_root` is not in `ctx.mapping` at all. **Result: the agent was handed `env_remote_run`/`push`/`pull` pointed at a far side, and not one `AGENT_SYS_*_REMOTE` variable saying where it is.** The comment at `prepare.py:528-538` records that this was the configuration the accepted remote run used — **live, not latent**. The instance is fixed. **What is recorded here is the class**, because the fix was one call site and nothing prevents the third: a question with two answer sources, where one source is *nearly* right, fails by omission rather than by raising, and omission is what `AGENT_SYS_*_REMOTE` does — no variable, no error, an agent that improvises a path. **What would close it**: `_far_side(ctx)` at `prepare.py:621` already exists and reads *both* fields; if every consumer of "where is the far side" went through it, there would be one reader. Nobody has checked whether any consumer still does not | + ## Unowned, reported more than once, recorded so they do not go stale silently Each of these has been raised by a package that does not own it and has stayed @@ -42,6 +48,16 @@ unclaimed. **Not blocked on anyone — nobody has them.** | 4e | **A hole in the store has no reaper** | §4.14 makes holes permanent and never renumbered by design. Whether they should ever be collected is undecided, not deferred | | 4f | **`check_grounded` has never been observed catching anything** — *ruled parked 2026-08-29, deliberately not worked* | Criterion 10 aims to show a validator catching an ungrounded number; three end-to-end runs showed a good model **declining to fabricate one** instead, so the validator's **failing** direction — what its `strong` claim is about — has never executed. **The user's ruling: not a framework question and not a principle question, this is `check_grounded`'s own business semantics, and it is not worth the time.** The shape they suggested if anyone ever picks it up: **split it in two** — one validator over the other fields, and a second that judges only whether the agent's answer about the missing duration is *reasonable*, passing if it is. **Two measurements bear on any such build:** `check_grounded` matches `\d+`, *"digits, not a parser"*, so `256` reads as grounded via `sha256_prefix` — the grounding set is **wider than what the facts assert**, and a fabricated number landing inside any digit run in the copied facts passes anyway. And `logic/check_grounded/readme.md` named the `UNEXPECTED_SUCCESS`/exit-3 outcome in advance, so **exit 3 is the artefact working, not a fault to repair** | +| 4g | **The `assets/` mechanism resolves entry points and pretends to be a resource mechanism** — *user-owned, small-scope refactor wanted (PR 155 review, 2026-09-04)* | `spec_loader/assets.py` finds **one file per role**: `body.readme` and `body.entry`, by filename convention, scoped by an optional `[.]/` folder. **Every other file an object needs is carried by nothing.** They arrive because `layout.stage_package(include=None)` copies the **whole package** into the zone, and a body reaches them by hand-built path — `exec python3 "$AGENT_SYS_TASK_PACKAGE/assets/check_x.validator/check.py"` is the pattern in every shipped validator. So a validator's `check.py`, its `readme.md` and any shared `assets/lib/*.py` ride along on a copy that is not the assets mechanism, and `body.entry` is a **pointer, not a manifest**. Two consequences: the object's resource set is never named anywhere, so `TODO.md` 4a (naming a task's executable set) cannot be answered from the assets index; and each body re-derives the same path string, so a layout change breaks them one by one at run time rather than at load. **The user's ruling: the mechanism is implemented badly and wants a small-scope refactor** — not a rewrite. Surfaced when `agent` needed its *directory* rather than a file and `fill_body` had no answer, which is `assets.py`'s own recorded gap (*"two of the four kinds have no body — a gap, not an omission"*), closed for `agent` by `resolve_folder` while leaving the resource question untouched | + +## Blocked on another change landing + +| # | Item | What closes it | +|---|---|---| +| 4h | **`env_mgr` spec §9.1's shared root has no constant in this tree** — *waiting on PR 154 (`dev.yihou.aiopt.more.demo`), 2026-09-04* | §9.1 states the rule — a declared install lands in one shared root, and only a `.claude/` tree is per-agent — and names PR 154's `AGENT_SYS_HOME` (`~/.infera_agent_sys`, `bin/ share/ state/ run/`) as its single owner. **That module is not in this branch**, so today's installs pin their destinations one variable at a time (`UV_TOOL_DIR`, `UV_TOOL_BIN_DIR`, `UV_CACHE_DIR`, and serena's `SERENA_HOME`), each into a scratch path chosen by the caller. **Deliberately not re-implemented here**: a second root would be exactly the parallel mechanism `engineer_principle.md` §2 forbids, and the two would drift over which one is authoritative. When 154 merges: take the path from its module, repoint those four pins under ``, and delete the per-caller choice. Nothing about the rule changes — only where the string comes from | + +| 4i | **User-level AI material outlives the run that declared it** — *surfaced 2026-09-04, needs an owner decision, not a patch* | `env_mgr` spec §9.1 sends a `.claude/` tree declared in `main.yaml`/`default.yaml` to **user level**, i.e. the agent_sys root's Claude config. PR 154 puts that root **deliberately outside any run root**, because a resident daemon has to outlive a single run. Both decisions are right on their own and their product is that **a task package's skills, hooks and MCP declarations persist into the next run of a different package.** Nobody chose that; it fell out. Three shapes are available and they are not equivalent — scope the material to the run and give up daemon-visible continuity; keep it and accept cross-run bleed as the meaning of *user level*; or add a third scope between them, which is the parallel hierarchy `engineer_principle.md` §2 exists to prevent. **Recorded before implementation rather than discovered after**, and it is not worked around silently | + ## To build in the alpha | # | Item | Note | diff --git a/agent_sys/docs/spec.provisioning.md b/agent_sys/docs/spec.provisioning.md new file mode 100644 index 000000000..ceb1bbe79 --- /dev/null +++ b/agent_sys/docs/spec.provisioning.md @@ -0,0 +1,212 @@ +# Provisioning — how an agent gets its environment + +| | | +|---|---| +| Status | **Normative for this round.** Written 2026-09-04 from the owner's rulings on PR 155 plus measurement | +| Scope | Everything installed or declared for an agent: recipes, add-ons, MCP servers, tools, skills, hooks, plugins | +| Spans | `env_mgr`, `agent`, `spec_loader` — which is why it is here and not in one component's `docs/` | +| Supersedes | the L1/L2/L3 vocabulary, entirely. There are no levels | + +**Every rule below has either an owner ruling or a measurement behind it, and the +measurements are named.** Where the rulings underdetermined something, the +derivation is shown rather than the conclusion asserted. + +--- + +## 1. The one rule everything else follows from + +> **Declarative first, and separate processes over shared ones.** +> +> A thing is installed by **declaring it in a recipe**. It is delivered by a +> **file the harness reads** or a **process of its own**. Python code that adds +> capability to a running agent, and code that runs inside the `agent_sys` +> process, are both exceptions requiring justification (§6). + +The one thing that is *not* installed by a recipe is an agent's own `.claude/` +tree, which is copied. §3. + +## 2. Recipes come in three layers, and the layer is where the file is + +| layer | where | how it is found | +|---|---|---| +| **default** | `env_mgr/default.env_recipe.yaml` | never named; always applies | +| **task package** | `/assets/main.env_recipe.yaml` | auto-detected, one fixed spelling | +| **agent** | `/env_recipe..yaml` | auto-detected, any `_stems` permutation | + +**There is no `layer` field on an item and there must not be one.** The layer is +carried by the path, and a field restating it would be a second writer of one +fact. A recipe carrying a stale `layer:` key is **rejected** with a dated +migration message (`recipe.py:73`) rather than silently passed into `Item.spec`. + +**`env_mgr/recipes/*.yaml` are demos** — the namespace of things you *name* in +`recipes: [x]`. The default is the one you never name, which is why it is not in +that directory. + +### 2.1 They concatenate; they do not override + +Default → package → agent, in that order, **additive**. A more specific layer +*adds* items; it does not replace them. Re-running an install is cheap because +every installer gates on `check` before `install`. + +**Nothing detects a version conflict between layers**, and this is a known gap, +not an oversight: `detect_conflicts` is scoped to one `run()` and `_run_recipe` +spawns one child process per recipe file, so three layers are three independent +checks. Closing it means parsing all three in the parent — the in-process +coupling the subprocess design exists to avoid. Also measured: `detect_conflicts` +fires only on **incompatible version constraints**, never on a repeated name, so +two layers both declaring `uv` is not an error and should not be. + +### 2.2 Absence + +**Declared and absent is an error. Undeclared and absent is simply absent.** +(`material.py:62-86`'s existing rule.) There is no third case. Both agent-level +systems — its own recipe and its `.claude/` tree — may be absent independently. + +## 3. Where an installed thing lands + +Two questions, in order. Neither is declared; both are derived. + +> **1. Is it AI material — a `.claude/` tree the agent harness reads as its own +> configuration?** If not, it installs **system-wide**, once. +> **2. If it is: did the *agent* declare it?** Yes → **project level**. +> Declared by the task package → **user level**. + +| what | where | Claude Code calls it | +|---|---|---| +| binaries, language packages, OS packages | system-wide; the agent_sys root where the installer accepts a prefix | — | +| a `.claude/` tree from `main`/`default` | the agent_sys root's Claude config | **user level** | +| a `.claude/` tree under an agent's own assets | the agent's workspace root | **project level** | + +**These are Claude Code's own two scopes and adopting them is the point.** A +harness that already distinguishes user from project does not need a second +hierarchy laid over it. + +**The copy route is for row three only.** Everything else is installed by a +recipe. + +Two consequences, stated because they are behaviour and not restatement: user +level is **shared across a run's agents**, and it **outlives the run** (the +agent_sys root is deliberately outside any run root — see `TODO.md` 4i). + +## 4. Add-ons + +`env_mgr/addons//` — what `agent_sys` itself ships for agents. **Inside +`env_mgr`, not beside it**, because `package-data` needs an owning package; +proven by building a wheel and counting members, not by the build succeeding. + +**Installed only by recipe.** There is no declaration key on `AgentSpec`. A +recipe locates an addon by importing `env_mgr` — `PYTHONPATH` is pinned to the +package root and `run_cmd` inherits it — so no path pointing outside the zone +needs exporting. + +## 5. MCP servers and tools + +**The transport decides the mechanism**, and it is not a preference: + +| transport | who starts it | how it is declared | +|---|---|---| +| **stdio** | **the harness spawns it** — that is what stdio means | a `.mcp.json` entry | +| **port-based (HTTP/SSE)** | **`env_mgr`, via the `run_server` installer** | a recipe item | +| in-process | — | **see §6** | + +`run_server` maintains a registry at `/servers.json`, keyed to the +run's lifetime. A duplicate declaration finds the entry and reports **`warn`** +without starting anything. Servers are stopped when the run ends. + +**The guarantee is *"stopped on normal and handled-error exit"*, not "always".** +`SIGTERM` has no handler and `SIGKILL` cannot have one. `PR_SET_PDEATHSIG` was +measured to close the `SIGKILL` case and is the wrong tool at the spawn site, +because the spawning process is a recipe child that exits within seconds. +`TODO.md` 4j carries both halves. + +A port already held: same binary → `warn`, different → `fail`, **and a holder +owned by another uid → `fail`**, because its command line cannot be read at all +and therefore can never be "basically the same". The identity key is the +**declared program token from the item's own `command`**, matched against the +holder's `/proc//cmdline` — asking *"is this the thing I was about to +start?"* rather than *"what is this process?"*, which has no answer when every +candidate reports as `python3`. + +### 5.1 `.claude/` and the SDK overlap, and how that is resolved + +Measured from the installed `claude_agent_sdk`: + +- **`setting_sources`** defaults to loading **all** filesystem sources (user, + project, local). `[]` is *SDK isolation mode*. +- **`strict_mcp_config=True`** ignores everything the CLI would otherwise load, + *"e.g. project `.mcp.json`, user/global settings, plugin-provided servers"*. +- **Same-name collisions have no SDK-level arbitration.** The fields are plain + dicts. + +So the overlap is **real, known to the SDK, and resolved by switches rather than +by precedence** — additive by default. Because the SDK arbitrates nothing, +**whoever merges two sources owns the collision**: `claude_sdk.py:375-386` names +it and refuses rather than overwriting, since the model addresses servers as +`mcp____` and a silent replacement makes one side's tools vanish. + +**Derived, and it resolves a claim that looked contradicted:** `.mcp.json` is a +**project-scope** filename — the CLI and SDK both say *"project `.mcp.json`"*, +and `--mcp-config` exists precisely to load one from elsewhere. A zone's +`$CLAUDE_CONFIG_DIR` is **user** scope. So `agent_assets.py:287`'s *"placing it +would put a file in the zone that nothing reads"* and the SDK's *"the CLI would +otherwise load project `.mcp.json`"* are **about different locations and both +true**. If the declarative route is ever wanted for MCP, its destination is the +**workspace root**, not the config directory. + +## 6. Two things that need justification, and one exception + +Both rules are about one temptation: reaching for Python because it is nearer +than a recipe. Neither forbids; each requires the reason to be written and to be +*"the other routes do not work"*. + +> **Adding an MCP server or a tool to an agent from Python code** needs a +> justification that no declarative route works. +> +> **Running an MCP server inside the `agent_sys` process** needs a justification +> that no separate process works. + +The second is the stronger: an in-process server is third-party or cross-module +code executing in the process that supervises every agent, with its memory, file +descriptors and credentials. There is no boundary to fail closed, and +`Installer`'s contract cannot express delivering one — it returns `Outcome`s, and +a live Python object does not survive a subprocess. + +**The standing exception** is `env_mgr/remote/tools.py` — `env_remote_run`, +`env_remote_push`, `env_remote_pull`. It is delivered by **injection**: +`claude_sdk.py:393` puts a live `create_sdk_mcp_server` object into +`ClaudeAgentOptions`, and **nothing is written to disk**, so no installer can +carry it. It works and has a live user. **Closing condition**: reprovide the +three as a standalone server started by `run_server`, after which this section +has no exception. See `ROADMAP.md` §6, *"If `agent_sys` must ever serve MCP +itself"*, which carries both halves: the owner's sketch for the case where +`agent_sys` would have to be the server, and the argument that `run_server` +already suffices — which is why the closing condition above is reachable today +and is not waiting on that sketch. + +**A second exception added by analogy to this one is the first rule being +ignored.** The point of writing it down is that the next case argues on its own +merits. + +**The in-process `ToolDef` route for *component-supplied* tools is deleted.** An +add-on ships a server that runs on its own. + +## 7. How an agent knows it is working remotely + +Not from prose. `prepare.py:645` returns the three remote tools if the zone has a +far side and `()` if it does not — **whether `env_remote_run` is in the toolbox +is the answer** — and `AGENT_SYS_*_REMOTE` mirrors every local path name. + +The reason this is a tool surface rather than a described procedure is in the +module's own docstring: *"an agent given a natural-language description of how to +sync a directory will improvise, and the improvisation will be wrong in a way +nobody notices."* + +## 8. What is deliberately not settled here + +- **`env_mgr/recipes/*.yaml` do not ship in a wheel.** `recipes: [serena]` in + `examples/env_checker` cannot resolve from a wheel install. Recorded in + `temp/bugs/2026-09-04-*`; the one-line `package-data` candidate is unrun. +- **Cross-layer version conflicts** — §2.1. +- **A registry sweep for runs killed by a signal** — `TODO.md` 4j. +- **Installs run unconfined** and §4 of `env_mgr`'s spec does not say so — + `TODO.md` 4k. Documentary, arguably required, unchanged. diff --git a/agent_sys/engineer_principle.md b/agent_sys/engineer_principle.md index e167aafb2..61deaf0b0 100644 --- a/agent_sys/engineer_principle.md +++ b/agent_sys/engineer_principle.md @@ -42,6 +42,7 @@ no good home, that is a finding to report, not a reason to pick the nearest file | MUST | | |---|---| +| **Never add a responsibility an existing mechanism can already cover** | Before introducing a concept, find the mechanism that already owns that job and **change it**. Modifying, clarifying or narrowing an existing component's responsibility is the normal move; adding a parallel one is not. A new module or concept is admissible only after a full analysis shows no existing component can host the job, **and** that its responsibility conflicts with nobody else's — and the analysis is written down, not asserted | | **Name the owner before writing the code** | "Which object is this a fact *about*?" usually answers it in one sentence | | **Never put something in a semantically wrong module because it is convenient** | Convenience is a one-time saving. A misplaced concept is paid for at every later read | | **Never fuse two modules to avoid deciding** | A merge is easy and a split is expensive. When in doubt, keep them apart | @@ -52,6 +53,20 @@ no good home, that is a finding to report, not a reason to pick the nearest file adding a thing at all so that some *other* module can compute with it.** That is §3, and §4.4 is what it looks like in practice. +**A second concept covering a job the system already has a mechanism for is the +same failure wearing a friendlier face, and it is harder to see because nothing +is obviously in the wrong place.** Two mechanisms for one job do not merely cost +twice; they drift, and every later reader has to learn which one is authoritative +in which case — a question the code cannot answer for them. **More concepts make +more to maintain**, and the maintenance is paid by whoever is not in the room +when the second one is added. + +So the order is: **find the owner, change the owner, and only then consider a new +component.** "The existing mechanism does not quite fit" is the beginning of the +analysis, not its conclusion — most of the time the right change is to the +existing mechanism, and the reason it looked unfit was that its responsibility had +never been stated precisely enough to argue with. + --- diff --git a/agent_sys/env_mgr/README.md b/agent_sys/env_mgr/README.md index e4867ab75..ac5cf9862 100644 --- a/agent_sys/env_mgr/README.md +++ b/agent_sys/env_mgr/README.md @@ -1,6 +1,6 @@ # env_mgr -Layered environment manager for the agent work system. Driven by one +Environment manager for the agent work system. Driven by one self-contained YAML recipe, it can **check / dry-run / install / bootstrap** an environment (Python, apt, binaries, Claude plugins/MCP) and report per-item status plus delivered artifacts (path/version/deps). @@ -36,16 +36,18 @@ Exit code: 2 on any FAIL, else 0. ## v1 limitations -- **Cross-layer skip-with-warning is not implemented** (design §4.1). env_mgr - does not yet walk the parent chain to detect that an item is already - satisfied by an upper layer and skip it with a warning. Each installer's own - idempotent `check` covers the practical single-host case. Consequently - `--on-conflict weak` is a **v1 no-op**: it skips cross-layer conflict - detection entirely and proceeds with install (exit 0), whereas `fail` - records the conflict and halts before install (exit 2). Only cross-layer - *version-conflict detection* under `fail` is active. -- **workspace layer is stubbed** — the default `$HOME/workspace.infera.aiopt` - path, its warning, and user-bin symlinking are not wired up yet. +- **Skip-with-warning is not implemented** (design §4.1). env_mgr does not yet + detect that an item is already satisfied elsewhere and skip it with a + warning. Each installer's own idempotent `check` covers the practical + single-host case. Consequently `--on-conflict weak` is a **v1 no-op**: it + skips conflict detection entirely and proceeds with install (exit 0), whereas + `fail` records the conflict and halts before install (exit 2). Only + *version-conflict detection* under `fail` is active. (This bullet described + walking a chain of *layers*; the layer model is gone — `docs/spec.md` §9.1 — + and what is unimplemented is the skip, not the chain.) +- **the workspace default is stubbed** — the default + `$HOME/workspace.infera.aiopt` path, its warning, and user-bin symlinking are + not wired up yet. The default appears nowhere in this tree. - **system apt is detect-and-print only** — the `apt` installer never runs sudo; it prints the `apt-get install` line for you to run. @@ -58,8 +60,9 @@ now lives entirely in this recipe and the installers above. # Above the wall: paths, zones, isolation -Everything above is the **shipped installer machinery** and is unchanged -(spec §9, criterion 22). Everything below is `docs/design.md` §2's subtree: +Everything above is the **shipped installer machinery**, reused rather than +reimplemented (spec §9, criterion 22 — no longer *frozen*: the layer model was +removed from it on 2026-09-04). Everything below is `docs/design.md` §2's subtree: `meta.py`, `fs/`, `isolation/`, `grants.py`, `workspace.py`, `material.py`, `sync.py`, `remote/`, `prepare.py`, and the CLI's two new sub-commands. @@ -701,6 +704,17 @@ kinds stay five and the zone-path kind grows from one name to six. one fact may not have two writers and `tests/cli/test_isolation_shown.py` imports it from there. +**Per-agent components did not add a sixth kind either**, and the reason is +worth stating because it looks like one. `agent_assets.install` contributes +`AGENT_SYS_AGENT_ASSETS`, and it reaches `Prepared.environment` *through* +`material.deploy` — the fifth contributor, whose whole job is already *what this +agent needs*. What components genuinely added is not a sixth environment source +but **two destinations that are not an environment at all**: +`Prepared.mcp_servers` and more entries in `Prepared.tools`. That is why +`material.deploy` returns a `Deployed` value now instead of a `dict[str, str]` — +the mapping had nowhere to put a nested server declaration or a live Python +object, and a second function returning them would be one act split in two. + | variable | value | the user's name for it | |---|---|---| | `AGENT_SYS_MY_ZONE` | `` | — (see below) | @@ -709,8 +723,27 @@ imports it from there. | `AGENT_SYS_MY_PLAYGROUND` | `/playground` | `my_agent_playground` | | `AGENT_SYS_MY_HANDOFFS` | `/handoffs` | — (`等等`) | | `AGENT_SYS_MY_LOGS` | `/logs` | — (`等等`) | +| `AGENT_SYS_AGENT_ASSETS` | `/package/` | — (added with per-agent components) | +| `AGENT_SYS_INSTALL_REPORT` | `/logs/agent_assets.install.json` | — (ditto) | | `_REMOTE` | the same path under `sync.remote_root(zone, mapping)` | `*_romote` | +`AGENT_SYS_AGENT_ASSETS` is **the one name in the family whose value is not a +zone subdirectory**, so `paths.py` owns its spelling and `agent_assets.install` +binds it. It still obeys the family's rule — exported and granted agree — because +the staged package is inside the zone and `prepare` grants the zone recursively. +It is not derived from `AGENT_SYS_TASK_PACKAGE` by a body, because the relative +part is the agent spec's and a body has no route to an agent spec. + +**Every name in this table is a path inside the zone**, and that is now +without exception. `AGENT_SYS_ADDONS_ROOT` used to be here, naming +`agent_sys/env_mgr/addons/` and defended as *the same rule run the other way* — +`isolation/policy.py::addon_grants` composed a `READ_EXEC` grant on it under the +identical condition that emitted the name. It went with the `agent_plugins:` +declaration key (`docs/spec.provisioning.md` §4): an add-on is installed by a +recipe, the recipe runs unconfined and copies what it needs into the zone, and +nothing confined reaches back out. Deleting the last exported out-of-zone path +is what the removal was for. + A name whose directory does not exist is **not exported**: the zone's subdirectories are one per registered domain kind, so a run with no `PLAYGROUND` domain has no `/playground`, and naming it would instruct a body to use a @@ -884,7 +917,7 @@ which half. | 19 | agent works on a copy; the stored artefact is unchanged | `test_agent_works_on_a_copy`, `test_stored_artefact_byte_identical`, `test_copy_out_refuses_to_copy_onto_itself` | | 20 | shared object store, main checkout unmodified — **D1**, not "is a worktree" | `test_workspace_shares_object_store`, `test_main_checkout_unmodified`, `test_the_agent_can_commit`, `test_collect_returns_work_by_a_supervisor_side_fetch`, `test_cut_refuses_a_main_repository_without_precious_objects`, `test_precious_objects_blocks_the_prune` | | 21 | conventions from a knowledge handoff, no code change | `test_conventions_come_from_a_knowledge_handoff`, `test_a_missing_knowledge_handoff_is_the_empty_default`. **The consumption half only** — the system-level task that would *produce* one is unspecified, so the test builds the artefact. The design recorded this as untestable; it is half-testable | -| 22 | the shipped machinery is untouched | The shipped **65**, unchanged, plus `test_the_shipped_modules_are_byte_identical` (asserted against the git index, not against memory) and `test_cli_subcommands_preserve_shipped_shapes` | +| 22 | the shipped machinery **keeps working** | `test_cli_subcommands_preserve_shipped_shapes`, plus the machinery's own tests. **Revised 2026-09-04** (`fc200a2`): the criterion read *untouched*, and a test — test_the_shipped_modules_are_byte_identical, named here **without backticks on purpose**, because `test_every_test_the_readme_cites_exists` scans backticked `test_*` names and cannot tell *citing a test as cover* from *naming one that was removed* — asserted that literally, over `git diff HEAD`. That was a scope fence for the round that built the new subsystems, and this round is a design-level change to the machinery itself, so the fence is retired. The **65** is a 2026-08-30 snapshot, not a live count. See `docs/spec.md` §10 criterion 22 for the full reason | **Beyond the criteria**, three suites hold properties nothing else would catch: `test_imports.py` (the decoupling wall, both directions, plus `fs/path.py` diff --git a/agent_sys/env_mgr/__init__.py b/agent_sys/env_mgr/__init__.py index 326d31c09..05a69217c 100644 --- a/agent_sys/env_mgr/__init__.py +++ b/agent_sys/env_mgr/__init__.py @@ -1,5 +1,5 @@ # SPDX-License-Identifier: MIT # Copyright (c) 2026, Advanced Micro Devices, Inc. All rights reserved. -"""env_mgr — layered environment manager for the agent work system.""" +"""env_mgr — environment manager for the agent work system.""" __version__ = "0.1.0" diff --git a/agent_sys/env_mgr/addons/README.md b/agent_sys/env_mgr/addons/README.md new file mode 100644 index 000000000..f7ca7d6b4 --- /dev/null +++ b/agent_sys/env_mgr/addons/README.md @@ -0,0 +1,124 @@ +# `agent_sys/env_mgr/addons/` — what this repository ships for agents + +An **add-on** is a capability `agent_sys` defines once — an MCP server, a skill, +a hook — so that more than one task package does not carry a private copy. + +**There is no declaration key, and that is the design rather than a gap.** +`agent_sys/docs/spec.provisioning.md` §4 is normative: an add-on is installed by +**declaring it in a recipe**, like everything else that is not the agent's own +`.claude/` tree. A recipe finds this directory by importing `env_mgr` — + +```yaml + - installer: embed + importance: required + name: envchk-baseline-server + tags: [internal, envchk] + run: | + set -eu + src="$(python3 -c 'import env_mgr, os; print(os.path.join(os.path.dirname(env_mgr.__file__), "addons"))')" + ... +``` + +— which works from a git checkout and from a wheel alike, because `addons/` is +`package-data` **inside** `env_mgr` (`pyproject.toml`) and because +`agent_assets._child_env` pins `PYTHONPATH` to the package root, which +`installers/base.py::run_cmd` inherits. + +## What changed, and why the previous shape is gone + +There used to be an `agent_plugins: []` key on the agent spec that named a +directory here and copied its whole `.claude/` tree into the zone. It is +deleted — the key, its JSON-schema property, `isolation/policy.py::addon_grants` +and the exported `AGENT_SYS_ADDONS_ROOT`. + +**Deleting the grant was the point.** `AGENT_SYS_ADDONS_ROOT` was the only path +`env_mgr` exported that pointed *outside* the zone, and it needed a `READ_EXEC` +grant to be usable at all. A recipe needs neither: installs run at `prepare` +step 6b, before any confinement is applied, so a recipe reads this directory +unconfined and **copies what it needs into the zone**. Nothing the confined body +touches is outside it. + +| what | how an agent asks | where it lands | +|---|---|---| +| something the industry ships — serena, a marketplace plugin, an apt/pip tool | `recipes:`, or the package / default recipe layer | wherever the installer puts it | +| something **this repository** ships — a directory here | the same: a recipe, whose item carries `tags: [internal]` to mark it ours | wherever that recipe copies it | +| something **one task package** carries for one agent | nothing — auto-detected at `/.claude/` and copied | the zone's `config/` | + +Only the third row is a tree copy. `spec.provisioning.md` §3. + +### `tags: [internal]` marks provenance and does nothing else + +Nothing was added for it. Verified first-hand against the tree, not recalled: +`Item.tags` exists (`env_mgr/recipe.py`), `tags` is in `_CLI_KEYS` so it is +excluded from `Item.spec` and cannot leak into an installer's arguments, +`--tag` is already a CLI flag (`env_mgr/cli.py`), and `env_mgr/runner.py` +selects on tag intersection. So `env-mgr install --tag internal` works today +with no schema change. + +**What the tag does not do:** it does not place a file, it does not change +grants, and no installer reads it. + +## The contract + +``` +agent_sys/env_mgr/addons// +├── README.md what this gives an agent, what it costs, and what it does NOT do +└── .claude/ the payload, in Claude Code's own canonical layout + ├── servers/*.py a server a recipe copies into the zone + ├── skills// SKILL.md + └── hooks/ … +``` + +**`.claude/` is Claude Code's format, not ours.** A file here is placed by a +recipe, not parsed — `env_mgr/material.py`'s own words. Anything needing a +conversion step is in the wrong format. Keeping the harness's layout is what +lets the recipe's `cp` be a `cp`. + +**An `.mcp.json` does not belong here.** A stdio server is spawned by the +harness from an entry in the *agent's* `.claude/.mcp.json` +(`spec.provisioning.md` §5), and that entry names `--project`, `HOME` and other +values that differ per agent. Both add-ons' `.mcp.json` files were moved into +`examples/env_checker/assets/env_probe.agent/.claude/.mcp.json` for exactly that +reason. What stays here is the **payload** the entry points at. + +**There is no `recipe.yaml` here either.** It used to be found beside `.claude/` +and run by `agent_assets`; the key that found the add-on is gone, so an add-on's +prerequisites are declared in whichever recipe installs it. + +## Paths inside one + +A payload is copied into the zone before it is used, so **nothing in it may name +a path outside itself**. Where a declaration has to point at one of these files +it does so through `${CLAUDE_CONFIG_DIR}`, which `env_mgr` has already redirected +at the zone's `config/` (`env_mgr/material.py`). A hard-coded `/home//…` +works on exactly one machine and fails silently on the next, because an MCP +server that cannot start is reported as **a server with no tools** rather than as +an error. + +## What is here + +| add-on | gives an agent | costs | +|---|---|---| +| [`envchk-baseline`](envchk-baseline/) | one **stdio MCP server** whose single tool returns a nonce-derived token | one `python3` subprocess for the life of the session; no network | + +`envchk-baseline` exists to be this directory's worked example, and it is a real +one rather than a stub: `examples/env_checker` installs it from +`assets/main.env_recipe.yaml`, declares it in its agent's `.mcp.json`, runs it, +and its `check_capabilities_genuine` validator re-starts the server itself and +compares the token the agent reported against the token the server produces. + +`serena/` was here too and is deleted: it held only the `.mcp.json` that +registers the server, and that moved to the agent that wants it. The **install** +is `env_mgr/recipes/serena.yaml`, which is a recipe and not an add-on. + +## Adding one + +1. `mkdir agent_sys/env_mgr/addons//.claude` and put the payload in it, in + Claude Code's layout. +2. Write `README.md`: what an agent gets, what it costs to install, and what it + does **not** do. The third is the one a reader cannot reconstruct. +3. Add a row to the table above. +4. Write the recipe item that installs it, and **check `pyproject.toml`'s + `package-data`** — a leading dot is not matched by `*`, so a new dot-file at + the leaf of `.claude/` ships only if a glob names it. Count the wheel's + members; do not read the build's exit code. diff --git a/agent_sys/env_mgr/addons/envchk-baseline/.claude/servers/envchk_baseline_server.py b/agent_sys/env_mgr/addons/envchk-baseline/.claude/servers/envchk_baseline_server.py new file mode 100644 index 000000000..7eaedc884 --- /dev/null +++ b/agent_sys/env_mgr/addons/envchk-baseline/.claude/servers/envchk_baseline_server.py @@ -0,0 +1,178 @@ +#!/usr/bin/env python3 +"""`envchk_baseline` — one MCP tool over stdio, standard library only. + +Declared by `../.mcp.json`, which is the **external** MCP route: a config entry +naming a command, as opposed to `.claude/tools/*.mcp.py`, where the file's +location is the declaration. The two routes are what `examples/env_checker` +tells apart, so this file must not also be named `*.mcp.py` or it would be +picked up twice and the distinction would stop being measurable. + +**No third-party import, on purpose.** A `pip install mcp` here would make a run +of `examples/env_checker` depend on a package index being reachable, and the +failure mode of an MCP server that cannot start is *a server reporting no +tools* — a silent degradation, not an error. The protocol below is four +JSON-RPC methods and the standard library covers all four. + +The tool returns a token derived from this component's salt and the per-run +nonce; `check_capabilities_genuine` re-runs this exact file and compares. See +`../../README.md` for what that does and does not prove. +""" + +from __future__ import annotations + +import datetime +import hashlib +import json +import os +import sys + +#: This component's salt. It exists nowhere else in this repository, and that is +#: what makes the token evidence rather than a format. +#: +#: ENVCHK_SALT: 48d7f4c12e751bebb631ff42ffe54656 +SALT = "48d7f4c12e751bebb631ff42ffe54656" + +#: Which of `examples/env_checker`'s six capabilities this one is. +LABEL = "mcp_external" + +#: Which of the two install routes delivered it (`agent_sys/docs/ +#: spec.provisioning.md` §3). **This file is not copied with a `.claude/` tree**: +#: `agent_sys` ships it under `env_mgr/addons/envchk-baseline/`, and +#: `examples/env_checker/assets/main.env_recipe.yaml` — the package recipe layer +#: — copies it into `$CLAUDE_CONFIG_DIR/servers/`. Reported as `installed_by`; +#: the field was `level: "L2"` until 2026-09-04, when the declaration key that +#: made L2 a level was deleted. +INSTALLED_BY = "recipe" + +SERVER_NAME = "envchk_baseline" +TOOL_NAME = "envchk_report" + +#: Echoed back to the client when it does not state one. Any client that does +#: state one gets its own value back, which is what the specification asks of a +#: server that supports the requested version. +DEFAULT_PROTOCOL = "2025-06-18" + + +def token(nonce: str) -> str: + """`ENVCHK-