[Draft] agent_sys with agent config - #155
Draft
dorado269 wants to merge 67 commits into
Draft
Conversation
…amples/env_checker
An agent may now carry components, not only files. `rules`/`hooks`/`skills` are
lists of paths; a Claude Code component is a tree, so three keys were added to
the agent spec and `env_mgr/agent_assets.py` installs them:
L1 recipes: [...] an env_mgr recipe YAML (serena, plugins, apt/pip tools)
L2 components: [...] agent_sys/components/<name>/, shipped here
L3 undeclared <agent assets>/.claude/, auto-detected
L2 and L3 have one on-disk shape, so one installer serves both and promoting a
component is moving a directory. L3 is undeclared because a declaration would
be a second statement of what the directory already says.
L1 runs the shipped machinery as a SUBPROCESS -- `python -m env_mgr <stage>
<recipe> --json` -- and the CLI's argument surface is therefore a compatibility
surface. Four reasons, the last two not recoverable by an import:
1. the decoupling wall (spec S9) stays literally true; cli.py remains the one
named exception, and `tests/env_mgr/test_imports.py` is untouched by this
commit, which is the evidence;
2. it is not the importlib dodge, which hides an edge that still exists;
3. a child takes env=, so CLAUDE_CONFIG_DIR and the UV_* roots need no
process-global os.environ mutation -- unsafe in a threaded runner;
4. a child can be bounded; run_cmd has no timeout and cannot bound itself.
PYTHONPATH is derived from __file__: measured, the editable install on this
host resolves env_mgr to a different worktree.
Measured 2026-09-03 and encoded: plugin installs merge into settings.json, so
the settings document is written before the first install; a plugin is read
from its marketplace SOURCE path at run time, so a component's marketplace is
copied into the zone and the copy is registered; uv writes ~/.local/share/uv
unless the UV_* roots are pinned, so the declared env block is resolved before
the installs and applied again after.
`${VAR}` in a component's .mcp.json expands against the zone environment and an
unresolved name is refused -- unexpanded, the server fails to start and reports
as a server with no tools, naming no cause.
New exports: AGENT_SYS_AGENT_ASSETS, AGENT_SYS_INSTALL_REPORT, and
AGENT_SYS_COMPONENTS_ROOT, the last granted read-only by
isolation/policy.py::component_grants under the same condition that exports it,
so paths.py's exported-and-granted rule still holds by construction.
Carried to the backend: Prepared.mcp_servers and Assignment.mcp_servers, merged
into the SDK's options under the collision policy the env_mgr tool server
already had -- a name already present is a named BackendUnsupported.
agent/docs/design.md O4 is answered: the declarative settings.json surface is
canonical. Components forced the ruling because they had to be stored somewhere.
examples/env_checker exercises all seven capabilities end to end. It declares
no `skills:` or `hooks:` deliberately: declaring them would make the run pass
whether or not L3 works.
pytest agent_sys: 2189 passed, 3 skipped, 4 xfailed.
Signed-off-by: yihou <yihou@amd.com>
Signed-off-by: yihou <yihou@amd.com>
…ber of a .claude/ tree Six defects found after 45d2a12, four of them by review rather than by the suite. The first two mask each other and that is the lesson worth keeping: 1. plugin installs ran a BARE `claude`. Under the policy-derived PATH that reaches /usr/local/bin/claude 2.1.197 (npm, root) while the session runs the pinned 2.1.246 -- two builds sharing one CLAUDE_CONFIG_DIR. 2. the install child got NO PATH at all. material.deploy builds its mapping from scratch and harness._RESERVED excludes PATH, so `uv --version` answered rc 127 `uv: not found`. Fixing only 1 leaves the serena recipe failing at item 2, naming the toolchain instead of the cause. Fixing only 2 ACTIVATES 1: a child with a real PATH is exactly what makes the bare name resolve to 2.1.197. The safe-looking partial fix is the one that turns a loud failure into a silent wrong binary. `cli/environment.py:512-519` predicted this from the other side -- it worried the SESSION would run a build other than the installs'. This was the mirror image, and it arrived because new code looked a binary up by name while every existing consumer had been made to name it. The mismatch is structural: `shutil.which` in the supervisor sees ~/.local/bin, the derived child PATH does not, so only naming the binary fixes it. An absent CLI is a `fail` outcome, not a fallback and not a raise -- local to the point a marketplace needs registering, so no-claude-and-no-plugins stays a working configuration. 3. BLOCKER: _install_tree copied skills/ and plugins/ and nothing else, so hooks/ and servers/ were named by consumers and placed by nobody -- a SessionStart hook pointing at a missing script, and an MCP server reported `ok` with args[0] absent, which is the "server with no tools" failure _expand exists to prevent, arriving after _expand did its job correctly. Fixed by INVERTING the default. Everything under .claude/ is placed; the exceptions are _NOT_PLACED = {settings.json, .mcp.json, plugins/}, each a read or a relocation with its reason. A copy-list is an allow-list that fails silently on anything nobody thought of, and a component format is extensible by design. A skip-list fails loudly. Same ruling fixed a latent one: a bundled tools/*.mcp.py is registered at its PLACED path, having worked for L3 only because the staged package happens to be inside the zone. 4. the marketplace contained() check ran AFTER copy_out, on a path built from the author-controlled manifest["name"]. With "../../../ESCAPED" the tree was written outside the zone and only then refused. Now validated by contained_syntactically before anything is copied. 5. `assets:` and package-relative `recipes:` could climb out of the staged package. F-D18 had been applied to spec_loader's output, not to these two consumers -- and _tooldefs imports from `assets:` into the supervisor. 6. three tests could not fail, verified by mutation: the settings-ordering test read the file after install returned; the PYTHONPATH test inherited pytest's cwd; the third described a deleted function. Also: same-tree .mcp.json/tools/x.mcp.py collision now warns; probe citations name the build (2.1.246) they are evidence about, and the withdrawn ~/.claude.json md5 claim is replaced with the evidence that holds. Eight mutations now fail that previously passed. Verified against the real package: hooks/, servers/ and both MCP args[0] present in the zone. pytest agent_sys: 2199 passed, 3 skipped, 4 xfailed. Signed-off-by: yihou <yihou@amd.com>
…source
`_mcp_servers` was fixed to register a bundled server at its PLACED path;
`_tooldefs` still imported from the source tree, which is the same defect one
function over. For L3 the two are the same file so it looked settled. For an L2
component the source is `agent_sys/components/<name>/.claude/tools/...`, so the
supervisor imported THE REPOSITORY rather than the copy this attempt was pinned
to -- two consumers with one of them reading a path the run does not own, which
is the bare-`claude` defect's class. The module docstring's narrowing ("the
staged package or this repository's own components/") is the wording that made
it read as fine.
It also stops `prepare` writing `__pycache__` into the components registry: the
bytecode now lands in the zone and dies with it. That is a consequence, not the
reason, and it is why no `sys.dont_write_bytecode` is needed -- a process-global
of exactly the kind the subprocess route removed.
Enumeration stays on the SOURCE tree and only the read moves. `<config>/tools/`
accumulates every level's files as each is placed, so listing it would re-import
the earlier component's module under the later component's name and register its
tools twice. Both halves have a test, and both fail under mutation:
importing the source again, and enumerating the placed directory.
pytest agent_sys: 2201 passed, 3 skipped, 4 xfailed.
Signed-off-by: yihou <yihou@amd.com>
… test the PATH fix
Two findings from review, neither escaping the zone, both defeating a decision.
1. `market == ".."` copied the marketplace onto <config>/ ITSELF.
`contained_syntactically("marketplaces/..", config_dir)` normalises to "."
and returns config_dir unchanged, so the component's plugins/ was emptied
into the config root -- on top of settings.json and everything _place_tree
had just written -- and `claude plugin marketplace add <config>` registered
the entire zone configuration directory as a marketplace. The trailing
contained() passed, because config_dir is contained in itself.
The check answered "does it stay inside the zone"; the requirement is "is it
a single directory name". Both now, in that order, so each message says which
rule was broken. `market` is also the right-hand side of <plugin>@<market>,
so a value that is not a plain name is wrong twice over.
2. The base_env PATH fix -- the more severe of the two defects in 04e8f97 --
had no test that failed without it. Dropping `**(base_env or {})` left 46
passing: the test asserted only the NEGATIVE (PATH not echoed back into the
returned mapping, true either way) plus a property of _child_env, which was
never the broken half. Now the seam test captures the mapping install was
actually handed, and a new end-to-end test puts a real executable in a
directory named ONLY by base_env's PATH and drives a real oneline item
through the real `python -m env_mgr` child -- `uv: not found` reproduced.
Also: _place_tree's symlink handling is documented, including an asymmetry the
report did not have. Measured: a TOP-LEVEL symlink goes through copy2 and is
dereferenced; a NESTED one goes through copytree(symlinks=True) and is
preserved. The preserved case is the worse of the two -- a link out of the zone
is a path the confined session cannot follow, which is probe F's
installs-cleanly-never-loads shape again. Neither escapes the zone. Recorded
rather than changed; a decision is owed.
policy.py's probe F citation names the build (2.1.246) it is evidence about.
pytest agent_sys: 2208 passed, 3 skipped, 4 xfailed.
Signed-off-by: yihou <yihou@amd.com>
…component overwrite 99d3aea moved the tooldef *load* to the placed copy and took the module NAME with it. The placed path is <config>/tools/<basename>, identical for every component shipping the same file name, so two components each shipping tools/util.tooldef.py produced one module name twice: module names : ['..._5bd3e578cf62e6df_util', '..._5bd3e578cf62e6df_util'] alpha : get_type_hints RAISED NameError("name 'AlphaArgs' is not defined") report : ok, ok, ok, ok The second import replaced the first in sys.modules, so the first component's annotations resolved against the second's namespace. That is exactly the state the sys.modules registration exists to prevent, arriving one component later instead of at import -- and the docstring forbidding it was still in the file, false about its own example. Fixed by keying on `source`, which is unique per component, and keeping `path` as the load location. The two halves are not interchangeable: `source` identifies the module, `path` is only where the bytes are read from. The existing two-component test uses DISTINCT filenames and is blind to this; the same-basename case is now covered, with a forward-referenced annotation because without one both modules look fine and the test passes either way. Underneath it was a silent overwrite. Levels install in order and copy_out merges, so a member two components both ship ends up holding only the later one's bytes -- one component's artefact absent from the zone while its own report says ok. _mcp_servers already warned about this for a server NAME; it was unreported for a FILE. Now warned, per file rather than per directory so that two components both shipping skills/ stay quiet unless they ship the same skill. Precedence is unchanged: later wins, which is L1->L2->L3's rule. Note for consumers of the install report, per the level-vocabulary rule: this adds a new `warn` producer. Verified it cannot fire on examples/env_checker -- the L2 and L3 trees share no file, and the real package's report is unchanged. pytest agent_sys: 2211 passed, 3 skipped, 4 xfailed. Signed-off-by: yihou <yihou@amd.com>
… depth Decided rather than recorded, per review. copy_out's default is asymmetric and that is shutil's property, not anyone's decision: measured 2026-09-03, a TOP-LEVEL symlink goes through copy2 and arrives dereferenced, while one NESTED inside a placed directory goes through copytree(symlinks=True) and arrives still a symlink. Same input, two results, decided by depth. Resolving is the choice, and a measurement picks it rather than taste: contained(<zone>/preserved -> /outside, <zone>) = False contained(<zone>/dereferenced_copy, <zone>) = True So a preserved link out of the zone is a path the confined session cannot follow, while the report says it was placed -- probe F's installs-cleanly-never-loads shape wearing a different hat. "Copy into the zone, do not reference out of it" was already this module's rule for the marketplace, and a preserved link IS a reference out of the zone. copy_out gains `dereference: bool = False`. Additive: the default is unchanged, so every existing caller keeps the behaviour it was written against, and _place_tree is the only one passing it. The asymmetry is documented at both ends, because it is surprising and it is not ours. What it costs, stated in the docstring: a component that links to host content gets that content copied into the zone at prepare time on its author's behalf. Refusing links instead would reject a layout nobody has written yet and would leave the asymmetry in place for links that stay inside the tree. Tested at both depths, because the defect was that they differed -- a test at one depth would have passed throughout. Dropping `dereference=True` fails the nested case. pytest agent_sys: 2213 passed, 3 skipped, 4 xfailed. Signed-off-by: yihou <yihou@amd.com>
…produced
`Prepared.mcp_servers` goes supervisor -> backend and is written down nowhere
else, so the install report is the only artefact that can carry evidence of
WHICH servers a run got. Run 1's report carried three different shapes and only
one recoverable name -- and not on the route that needed it:
external "1 external MCP server(s) from ..." details {}
bundled "bundled MCP server 'envchk_stdio'" details {"path": ...}
in-process "1 in-process tool(s) from ..." details {"path": ...}
External is the only route whose keys an author chooses in a data file, so it
is the only one that can silently be about a different server -- and it
recorded a count. Bundled had its name only inside the message string or the
path stem, so reading it means re-deriving a name the producer already knew.
In-process recorded no name at all: neither `envchk_echo_token` nor the server
key it is addressed under.
Now all three record one, so a consumer comparing declared capabilities against
what installed is four rows of one rule instead of one row plus two special
cases:
external {"names": [...]}
bundled {"server": <stem>, "path": ...}
in-process {"server": "env_mgr", "tools": [...], "path": ...}
`tools` is the load-bearing key in the third, not `server`: the server name is
a constant every run has the moment any ToolDef exists, so a check against it
alone could not fail -- and a check that cannot fail is the shape caught five
times today. The tool name is the half of mcp__env_mgr__<tool> an author
chooses and the half that gets mis-addressed.
IN_PROCESS_SERVER duplicates `claude_sdk._TOOL_SERVER`, which env_mgr may not
import (interfaces.md 4.6). That is a second writer for one fact and the cost
is stated at the constant: this copy is only ever REPORTED, never used to
address anything, so a divergence makes the report wrong and breaks no call.
Scope, stated because it is narrow: this catches declared-nowhere at prepare
time, before the agent starts. It does not catch a server that is declared and
fails to start -- that is the args[0] existence assertion, or a handshake.
Also adopts `reviewer`'s 15-member closure fixture, which asserts the
place-by-default rule over SHAPES rather than remembered names: a top-level
file, an empty directory, a symlink, an executable whose mode must survive, and
a nested .mcp.json that is placed as data because only the top-level one is an
interface document. Adopted verbatim but for the symlink note, which pinned
current behaviour and has since been decided (c92063a).
pytest agent_sys: 2216 passed, 3 skipped, 4 xfailed.
Signed-off-by: yihou <yihou@amd.com>
…what an in-process tool cannot see
Two things, both scoped by the lead's ruling; the in-process route itself is
NOT changed because whether it should be run-aware is still open.
1. The generated bundled-stdio entry gains `env`.
This is not a fix for something broken. Measured in run 2 (capability 5):
the bundled server DID see the run's ENVCHK_NONCE with no `env` key at all,
because the SDK hands Prepared.environment to the CLI child and the CLI
spawns this server as its child. It is a fix for NOTHING STATING WHY IT
WORKS -- what makes it work is a third party's process model, one CLI change
from silent, and silent here means a well-formed answer computed against the
wrong environment, which is exactly what run 2 measured the in-process route
doing.
The WHOLE mapping, not a subset, under an unmeasured semantics: whether the
SDK merges this `env` with the child's or replaces it is not measured. A
subset is correct under merge and strips PATH under replace, so the server
stops starting; the full mapping is correct under both. Evidence points at
merge -- envchk-baseline declares only ENVCHK_NONCE and its server starts --
but that is an inference from one run and is recorded as one.
The harness block carries credentials and they already reached this server
by inheritance, so the exposure is unchanged and is now visible.
2. The in-process route's limitation is recorded at the loader.
Measured, driven through claude_sdk._adapt_tool with no model call:
supervisor pid=784037 ENVCHK_NONCE=None
tool saw pid=784037 thread='asyncio_0' ENVCHK_NONCE=<unset>
A *.tooldef.py's `call` runs in the supervisor, so it reads the supervisor's
os.environ; Prepared.environment reaches the CLI child and nothing else. The
security half of that sentence was already in the file -- package code runs
in the supervisor -- and nobody had connected the consequence.
Written as WHAT IS TRUE TODAY plus the open question, deliberately not as
"the supervisor's environment by construction". That phrasing would be a
claim about the future, and if the route is later made run-aware it becomes
false prose already shipped -- a reader cannot tell a live constraint from a
stale one. So it states the measurement, names binding-at-load as the
available-and-unbuilt option, and tells a component needing a run-specific
value to ship a *.mcp.py meanwhile.
pytest agent_sys: 2217 passed, 3 skipped, 4 xfailed.
Signed-off-by: yihou <yihou@amd.com>
An empty commit, because the fact it records has no file to live in.
run 20260903T103441-53acfd
package tree 13154c6
env_mgr 9a9fdff
Why the two SHAs differ. Run 2 launched with `env_mgr` at 9a9fdff and its
pins recorded that, which was true and NOT sufficient: the serena wiring,
self-test case 2 and the validator that produced the verdict were all
uncommitted working-tree edits at launch. They landed later, in 13154c6.
So the run's own log names one SHA and the package it executed is at
another, and without this commit nobody could pair them -- reproducing
from 9a9fdff yields a different package than the one that ran.
This does not make run 2 retroactively attributable. Nothing can. It stops
the window widening and it makes the pair recoverable by grep.
**A run is attributable to a PAIR of SHAs** -- the package that ran and the
`env_mgr` tree it ran against. That is the lesson run 2 bought, and three
pins (run id, nonce, HEAD) were never enough. `selftest/launch.sh` now
refuses to launch while `git status --porcelain` is non-empty for
`examples/env_checker` or `components/`, and `ACCEPTANCE.md` records it as
the fourth pin.
Why 13154c6 is a MIXED commit -- two files `env_mgr`, eleven the package --
and it is not carelessness. Four agents share this worktree and therefore
share one git index. `pkg-author` staged eleven package files with explicit
paths; `core-impl` committed its own `env_mgr` work in the same moment, and
`git commit` took everything the index held, including the eleven. The
symptom is silent: a successful commit containing someone else's changes
under your message, and it cuts both ways. 13154c6 is left standing rather
than reset -- a `reset --soft` mutates the shared index another agent may be
mid-`add` on, to repair an attribution error in an unpushed commit where no
content was lost. The cure touches shared state and the disease does not.
The rule adopted from it: `git commit -- <explicit paths>`, always, and read
the staged set back before committing.
Signed-off-by is the shared identity; authorship of the eleven package files
in 13154c6 is `pkg-author`'s.
Signed-off-by: yihou <yihou@amd.com>
…duct
Two things the run-2 incident bought, recorded where the next author looks
rather than in a thread.
`README.md` gains the instrument section. Six checks in this package and its
scratch tooling turned out to be checks that could not fail, and half were in
the instruments rather than in the package -- instruments get less adversarial
scrutiny precisely because they are the thing measuring. Two corollaries:
* A gate whose only self-test is a live launch will be tested by launching.
`selftest/launch.sh`'s dirty-tree gate was verified by running the script;
the gate passed and the script launched, starting a third run that
overwrote two of run 2's logs. The careful action and the destructive one
were the same command. It now has `--check`.
* Fixing one instance of a class does not inoculate you against the class.
`preflight.sh` erased its own hand-written verdict every run; that was
found and fixed by generating the verdict. The same bug -- a fixed filename
for a per-run artefact -- was then written into the neighbouring script and
destroyed run 2's launch log. The second instance arrives in the file
nobody is looking at, and having just fixed the first is what makes you not
look.
`.gitignore` gains `.serena/`. serena writes it into whatever directory it is
given as `--project`, and capability 7's measurements point it at the agent
asset directory: config and symbol cache, regenerated on demand, belonging to
no package. Ignored rather than remembered, because `launch.sh` refuses to
start while the package tree is dirty -- an untracked by-product everybody
knows to skip would block the gate and teach people to bypass it, which is the
worst thing that can happen to a gate.
No evidence was lost in the incident: run 2's handoff, its zones and its
install report are untouched, and the verdict is reproducible from the
artefact. Three log files were overwritten, and the pins survived because they
had been reported. The stray run directory is kept deliberately -- it is the
only on-disk record that this happened.
Signed-off-by: yihou <yihou@amd.com>
`selftest/launch.sh --check` runs both gates -- package committed, demo-root unused -- and launches nothing. The operating rule is to never invoke the script without `--check` for any purpose other than actually launching. Recorded in ACCEPTANCE.md as well as in the script's own header because the script lives in /tmp scratch, and a rule that exists only in the tool it governs disappears with the tool. The lead asked for the header comment 'so it survives you'; /tmp does not survive, so the durable copy goes here. Signed-off-by: yihou <yihou@amd.com>
…ction 3
Section 3 said, of the three re-derived rows:
"Rows 4, 5 and 6 are re-derived by the validator running the capability,
so their tokens have no file-read path."
The clause after the comma was false, and it was false about all three rows
rather than one. `replay_mcp` starts each server with
`env=dict(os.environ, ENVCHK_NONCE=nonce)` and `replay_import` sets the
variable in its own process before calling the handler -- the validator
forces the input in every one of them. A re-derivation therefore confirms
the salt and the arithmetic; it cannot confirm what the capability would
have seen, and it says nothing about how the AGENT obtained its answer.
Found while analysing row 6 after run 2, then found to apply unchanged to
rows 4 and 5, which nobody had looked at. That is this repository's own
rule -- fixing one instance of a class does not inoculate you against the
class -- landing on the person who wrote it down, hours later.
The old sentence is quoted in place and its false clause marked, rather
than the section being rewritten silently: a claim whose meaning changed
invisibly is worse than one that failed, and this package's whole subject
is claims that quietly stop being true.
The honest tiers, now stated as a table: rows 4/5/6 establish that the
artefact EXECUTES AND COMPUTES CORRECTLY -- it catches a server that will
not start, a module that will not import, a wrong derivation in the code.
Rows 1/2/3/7 establish that the artefact EXISTS AND IS REACHABLE. Neither
tier establishes that the agent obtained the token through the capability
rather than by reading the file, and none can while the agent and the
artefacts share a zone by construction.
Row 2 is promoted to its own bullet as the only row in the package with
genuine agent-side evidence. Its `session_id` and `hook_event_name` arrive
on the hook's stdin from Claude Code, so an agent that ran the script by
hand gets an empty payload and the validator says so. That was true all
along and was hidden by grouping it with the file-borne tier.
No behaviour changes. This corrects what the document claims about
evidence the package already produces.
Signed-off-by: yihou <yihou@amd.com>
…text Completes the record started in 13154c6, which stated only the negative -- what an in-process ToolDef cannot see. The rule an author actually needs: A tool's per-run context arrives as a TOOL ARGUMENT. It never arrives through the environment, because the tool runs in the supervisor. Forced by the process model rather than a style preference, and that is the part worth keeping: the supervisor is shared by every attempt and agent/runner.py is threaded by construction, so a tool reading ANY process-global state cannot be per-attempt even if the value were populated correctly. An argument binds per call; a closure binds per construction; the environment binds per process, and the process outlives the attempt. Same root cause as the rejected option of setting os.environ around the handler, not a second one. remote/tools.py is named as the shape done properly and explicitly labelled NOT REACHABLE FROM A PACKAGE, which is the honest form of a precedent you cannot follow: `tools(...)` is a factory, prepare calls it with the zone in hand, and each ToolDef.call closes over it -- while a *.tooldef.py exposes a module-level TOOLS and nothing ever hands that module anything. Checked before writing; pointing authors at the closure form would have named a pattern they cannot reach, which is a true sentence broader than what was checked, in prose. The open question is recorded dated and attributed to run 2: env_mgr's own tools would have this exact bug if they were declared the way packages must declare theirs, and are immune only because they get a call site -- so the missing piece is not documentation, it is the factory env_mgr already gives itself and does not give packages. Undecided at the time of writing, and worded to be correct either way: the record of what was rejected, or the specification of what was accepted. Also repairs the section's flow. The earlier insertion split the security paragraph from its own opening sentence, leaving "That is not a sandboxed act" dangling after an unrelated one. No contract change, no factory, nothing in agent/. pytest agent_sys: 2217 passed, 3 skipped, 4 xfailed. Signed-off-by: yihou <yihou@amd.com>
Run 2: the in-process ToolDef returned a token computed from an EMPTY $ENVCHK_NONCE. A ToolDef runs in the SUPERVISOR's process, and Prepared.environment -- which carries the agent spec's `env` block -- is handed to the CLI child. The supervisor never sees it. The tool did not fail; it returned a well-formed token about nothing and the agent quoted it correctly. The tool lied to the agent. Row 6 used to claim, implicitly, "the tool can see the run's environment". It cannot, and no test written here changes that. It now claims: the module was imported, executed, read a file and computed correctly. The tool's input is its own `__file__` and nothing else -- not the environment (broken for this route), not an argument (then the model chooses the input). `_tooldefs` imports the placed copy, so the path carries this run's zone identifiers while the bytes are identical every run: the freshness is in the PATH, not the content, and that is stated rather than left to be discovered. The token derives from that path for a second reason. A token byte-identical across two runs is the signature of the empty-nonce bug -- `scribe` proved the defect exactly that way -- so a fix reproducing the defect's fingerprint would be indistinguishable from the defect. 6b is its own row because its failure is a different KIND. Row 6 failing means one capability did not work; 6b failing means the isolation property is broken for every tooldef any package ever ships. It is what fails when a tooldef is imported from the component source instead of the placed copy -- e1b9f54's bug, which nothing else in this repository guards. The reader's next action differs: one sends them to the agent, the other to `_tooldefs`. All of 6b's discriminating power is in the path and none is in the digest: placed copy, staged source and working tree all digest to 39650f61fd050a3a. Making the digest discriminating would require the placed copy to differ from its source, breaking the property under test. The weakness is the design. The validator derives the expected path ITSELF from `Path($AGENT_SYS_TASK_PACKAGE).parent`, rather than accepting `proof.raw.path` and digesting whatever it names -- otherwise 6b reduces to "the tool checksummed the file it said it checksummed". It uses AGENT_SYS_TASK_PACKAGE and not AGENT_SYS_MY_ZONE because entry.sh already refuses to start without it, so a running body has it by the fact of running. `replay_import` no longer forces ENVCHK_NONCE into its own process. That forcing meant it could only ever catch a wrong salt, never a wrong environment -- the defect run 2 found -- and with the tool taking no input there is nothing to force. Also: the components-root cross-check against the install report is added and labelled CONSISTENCY, not independence. Both fields come from the agent, so it catches an inconsistent report and not a consistent misstatement. Signed-off-by: yihou <yihou@amd.com>
The tool returns `path` and `sha256` alongside `token`, and rows 6 and 6b check all three. The brief still described the old shape, so an agent following it would have omitted two fields the validator requires and failed an otherwise honest report -- a brief and an implementation disagreeing about an artefact, which is the failure this package keeps finding in itself. Also tells the agent not to compute the token from the salt: it derives from the module's own placed path, which the agent cannot construct. Signed-off-by: yihou <yihou@amd.com>
Run 3's `check_env_report_shape` refused a well-written deliverable. The
agent documented the token's format in its Schema section:
- `token` — the `ENVCHK-<LABEL>-<12 hex>` string, or `null` if not obtained.
and the `<...>` rule could not tell DOCUMENTING a placeholder from LEAVING
one. The agent did exactly the right thing; the check was wrong.
This is a different species from the seven defects found earlier the same
day. Those were checks that could not fail. This one FAILS ON CORRECT
INPUT, and the repair points the opposite way: narrow it, do not
strengthen it -- and do not delete it either, because an unfilled `<...>`
in running prose is exactly the sloppiness the rule was written for, and a
real check that misfired once is still a real check.
The rule is split. `TODO`/`TBD`/`FIXME`/`XXX` still match everywhere,
including inside code, where an unfinished thought is still unfinished.
The `<...>` form now runs over prose only, with fenced blocks and inline
code spans blanked first -- blanked rather than removed, so that nothing
either side is joined into a new match.
Two self-test cases, because a fix to a check needs the same treatment as
a check: the exact line run 3 died on must PASS, and a bare-prose
`<fill this in before shipping>` must still FAIL. The first fails without
the narrowing.
The same pattern is in four other files -- `single_real_task`'s
`check_packup_shape` and three validators in
`llm_e2e_performance_optimization`. Verified rather than assumed:
`single_real_task`'s copy flags the exact line run 3 died on. They are NOT
changed here. Three belong to a different deliverable with its own
acceptance history and one is the template; changing a validator in a
package we are not running changes an acceptance criterion for work
accepted under the old one, without re-running it. Recorded in the
package README as out of scope with that reason, together with the
mechanism: this defect propagates by copy, and each copy arrives carrying
the authority of the file it came from.
Signed-off-by: yihou <yihou@amd.com>
Four items, each carrying what would make it worth doing. An item nobody can tell has become due is a wish, not a deferral. 1. `6b` can become genuinely independent. Its install-report cross-check is CONSISTENCY only -- both fields come from the agent. Runs 3 and 4 established by measurement that a real validation zone does carry the environment, so the validator can read $AGENT_SYS_INSTALL_REPORT itself. Trigger already met; three lines in a body that already reads that file. 2. The placeholder regex in four other files. Trigger: whoever next runs one of those packages, or a decision to build the shared helper. The repair wants to be one shared change, which is why it did not happen as four copies here. 3. `agent_sys`'s in-process tool factory. Row 6 works around a contract, it does not fix one. Trigger: the SECOND package that wants a per-run value in an in-process tool -- one is a workaround, two is a missing feature, and the second is the evidence that justifies changing the contract. 4. A verdict is not attributable to a named validator from a run's artefacts. The artefacts say a validation failed; recovering which one and why needed a hand re-run, and for run 2 the reason was not recoverable from the files at all. Trigger already met, twice. Not an env_checker item -- recorded here because this is where it kept biting. Signed-off-by: yihou <yihou@amd.com>
dorado269
commented
Sep 4, 2026
dorado269
commented
Sep 4, 2026
dorado269
commented
Sep 4, 2026
| ) | ||
|
|
||
| if method == "tools/call": | ||
| called = request.get("params", {}).get("name") |
Collaborator
Author
There was a problem hiding this comment.
do not implement MCP from scratch
use standard lib
import httpx2
from mcp.server import MCPServer
Initialize MCPServer
mcp = MCPServer("xxx")
Collaborator
Author
There was a problem hiding this comment.
Not done, and I would rather say so than quietly close this. You are right about the shape — hand-rolled JSON-RPC is the wrong thing to ship.
What blocks it is the interpreter, not the code. These servers are launched as bare python3 <script> from inside a zone, so the interpreter is whichever python3 the zone's PATH selects — a system one, which has no mcp package. Verified: /usr/bin/python3 -c 'import mcp' raises ModuleNotFoundError. The failure mode is the bad one — the server exits at import and Claude Code reports zero tools rather than an error, with nothing in the run naming the cause.
Closing it means either having the recipe layer install mcp into the zone and pinning the server's interpreter to it, or launching servers through a resolved interpreter rather than a bare name. Both are wider than rewriting the two files, so it is recorded as docs/TODO.md 4m rather than guessed at. Leaving this thread open for you to rule on which route.
Edit: the row is 4m, not 4k — 4k was already taken. Its second claim is also marked as inferred there rather than measured: the missing mcp package is measured, but that an ImportError at start-up presents as zero tools rather than an error has not been driven for this case.
dorado269
commented
Sep 4, 2026
dorado269
commented
Sep 4, 2026
dorado269
commented
Sep 4, 2026
dorado269
commented
Sep 4, 2026
dorado269
commented
Sep 4, 2026
| # It is one name and not two on purpose. The point of L1 here is the *route*, | ||
| # and a second recipe would add a second way for the run to fail on somebody | ||
| # else's network without measuring anything the first does not. | ||
| recipes: [serena] |
Collaborator
Author
There was a problem hiding this comment.
recipes can also be defined in task_package.
and rename 'recipe[s]' to "env_recipe[s]"
- env_recipe.${agent_name}.yaml or ${agent_name}.env_recipe.yaml is automatically detected in the whole test package.
- the task_package can have it own main.env_recipe.yaml or env_recipe.main.yaml . as the workspace level[kind] (If i am not wrong, there is a overlay/fallback system of different kind recipe like workspace/repo/project..., tell me details about that and we make sure how next step do.)
- embedded recipes define is not allowed. but you can specifically refer to an exist recipe.
- I don't think list/arr is needed here.
any unclear terms, please ask me.
Collaborator
Author
There was a problem hiding this comment.
Three of the four are done; the fourth is not, and I have left this open for that.
1. Auto-detection — done. env_recipe.<agent>.yaml, <agent>.env_recipe.yaml and any other .-joined permutation are discovered, including inside the agent's own <name>.agent/ folder where the name may be dropped. Scoped to assets/: a recipe at the package root is a load failure. Two spellings for one agent is SpecInconsistent.
2. The task package's own recipe — done. <package>/assets/main.env_recipe.yaml, that exact filename only, auto-detected and never declared. On the overlay question you asked about: the layers concatenate, they do not override. A later layer adds items; it never replaces an earlier one. That costs nothing because every installer gates on check before install, so an item two layers both declare is done once and reported twice.
3. Embedded definitions — done. recipes: accepts references only; an inline recipe object fails schema validation. Every reference names its root — agent_sys:<name> or package:<relpath> — and a bare name is refused with both spellings named.
4. The list — not done. recipes: is still an array. One agent can legitimately want more than one recipe, and collapsing to a single value would remove that with no replacement. But that is my reasoning, not your ruling, so I have not made the call: tell me whether you want a scalar and I will change it.
dorado269
commented
Sep 4, 2026
dorado269
commented
Sep 4, 2026
| @@ -0,0 +1,160 @@ | |||
| # SPDX-License-Identifier: MIT | |||
Collaborator
Author
There was a problem hiding this comment.
add a default.yaml recipe as the very default recipe if nothing defined for an agent in task package(no main recipe, no agent recipe).
- use most bottom level kind
- please add my current user claude plugins into it (see what I installed and use claude intsall ...)
- add cli
- install something else, make it good enough for claude code to works for a develop and experiment job working in repos like infera/sglang.
- print warning when using it for an agent
Collaborator
Author
There was a problem hiding this comment.
sorry, add gh cli
Collaborator
Author
There was a problem hiding this comment.
Partly done. The layer exists; its contents do not yet match what you asked for, so I am leaving this open.
Done: env_mgr/default.env_recipe.yaml is the default layer. It is at the bottom kind (workspace), it applies to every agent, nothing declares it, and its absence is simply absence. It sits in env_mgr/ rather than env_mgr/recipes/ so that the path says it is the default and no field has to.
Not done, and each needs a decision from you:
ghand a wider CLI set. I did not add an install command I cannot exercise here —ghalready resolves on this class of host, so the install branch would never run and could not be claimed as tested, and a rootlessghinstall is a real choice (distro package vs. release tarball) rather than a one-liner. Tell me which and I will add it.- Your marketplace plugins. Installing them needs
claude plugin marketplace add anthropics/claude-plugins-officialfirst, and that clone was observed going over SSH. A zone with no forwarded key fails it, and whether the CLI falls back to HTTPS is unmeasured. - The warning. Not implemented, and it interacts with a design point worth naming: you described the default as a fallback for an agent with no other recipe, but it is currently a layer that always applies and concatenates under the others. Those are different mechanisms and only one should exist. Which did you intend?
dorado269
commented
Sep 4, 2026
…rsed or not `assert [o["level"] for o in document["outcomes"]]` is a truthiness test on a list that is never empty: on a RecipeError `cli.main` still prints well-formed JSON carrying one `fail` outcome, so it was `["fail"]` — truthy — while the docstring says the test catches a recipe that resolves and does not parse. Asserts `status == "OK"` instead. Host-independent, and checked rather than hoped: all three of this recipe's items reach `plan`, and none of the three plan methods can emit `warn` or `fail` — ShellInstaller and BinInstaller return ok/info, UvInstaller's tool form returns info. Measured 2026-09-04 both ways: OK with levels info,ok,info on the shipped recipe, and FAIL on a deliberately broken copy where the old assertion stayed green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: yihou <yihou@amd.com>
Found by re-running the grep rather than trusting the one I ran during the rename: my first pattern was narrower than the word it was looking for, so it missed paths inside comments and quoted prose. Same class as the round's recurring one -- fixing instance 1 is what makes you not look for instance 2. Three were paths that no longer exist: env_mgr/recipes/serena.yaml's reference to components/envchk-baseline/.claude/ .mcp.json, and the validator's '../../components' in both check.py and its readme. The remaining four are prose naming the old directory or the retired L2 level: agent_assets.py x4 (including a quotation of its own earlier wording), ACCEPTANCE.md x2, and steps/check.yaml's comment. serena.yaml also now records why it carries NO tags: [internal]. Every item in it installs a third party's code, so tagging it internal would make the tag mean 'a recipe we wrote' rather than 'a component we own', which is the distinction it exists to draw. The half of serena that IS ours is agent_plugins/serena/. Written as a decision at the top of the file so the next reader does not add the tag as an oversight repair. tests/env_mgr + tests/spec_loader + tests/cli: 838 passed, 2 skipped, 2 xfailed. Signed-off-by: yihou <yihou@amd.com>
…cker's data The claim in my last report -- "the level vocabulary is gone from the core" -- was broader than what I had checked. It enumerated the files I had edited and then generalised. Measured properly this time, with the grep run over the whole tree rather than over my memory of it. tests/env_mgr/test_agent_assets.py had 12 sites, not the 4 in the list I was given -- 3 of them TEST FUNCTION NAMES (test_l1_/test_l2_/test_l3_), which no README cites, so renaming them is safe. One of those names also still read "repositorys_components_directory": my earlier grep used -w, and that string has no word boundary at "components", so a whole test name survived a rename that was looking for exactly that word. Eighteen tmp_path/"components" fixture directories renamed with it. examples/env_checker/steps/check.yaml had 9 comment sites, not 2. All 9 are about which install ROUTE is being declared, not about the "level" string the agent reports, so rewording them does not touch the acceptance contract. tests/spec_loader/test_assets.py:414 is one nobody listed: a docstring copied from spec_loader/assets.py, which I reworded at the source and not in the copy. DELIBERATELY UNCHANGED, and each for a stated reason: - envchk.Capability.level and the tables that document it -- that package's acceptance contract, its own change. - agent_plugins/envchk-baseline/.claude/servers/envchk_baseline_server.py -- LEVEL = "L2" is data the validator compares, so its comment and its tool description saying L2 are CORRECT beside it. Rewording only the prose would have made the file incoherent. - agent/docs/design.md:1300, a dated record of a past ruling. - env_mgr/agent_assets.py:24 and agent_plugins/README.md:16 say they WERE numbered L1/L2/L3. That is the retirement notice doing its job. env_mgr/recipes/serena.yaml:4 was already gone -- 75edb7c replaced that whole comment block. tests/env_mgr + tests/spec_loader + tests/cli: 838 passed, 2 skipped, 2 xfailed. ruff check and ruff format clean on all three files. Signed-off-by: yihou <yihou@amd.com>
…ship Found while checking whether the components/ -> agent_plugins/ git mv could break packaging. It could not, because neither name was ever packaged -- the check came back clean and the clean result was the bug. Measured by building the wheel and listing its 149 members: no agent_plugins/ member and no .yaml member of any kind. So recipes: [<bare name>], which _recipe_paths resolves to <site-packages>/env_mgr/recipes/<name>.yaml and which examples/env_checker actually declares, cannot resolve either. That second route has nothing to do with the rename and is the worse of the two. Both fail LOUDLY, as a named PrepareRefused, and the record says so rather than implying a silent wrong value. From a checkout, which is how everything runs today, both work. pyproject.toml already carries a comment describing this exact failure, written about spec_loader/schemas/ and fixed for that one directory. The class was not swept for; two more instances were beside it. Signed-off-by: yihou <yihou@amd.com>
Signed-off-by: yihou <yihou@amd.com>
Signed-off-by: yihou <yihou@amd.com>
… started A server's startup is declared in a recipe like anything else, and the run that started one stops it. - `installers/run_server.py` -- command, port, optional ready_timeout. "Started" is **alive AND bound**, polled to the timeout, so the two failures stay separate: a command that exits is reported with its rc, a process that never binds with the port it did not take - `servers.py` -- a JSONL registry keyed by an environment variable, and `owned_servers`, the context manager `cli/main.py` enters. The starter and the stopper are **different processes** (a recipe runs as `python -m env_mgr`, which exits seconds later), which is why the registry is a file and why the path travels in the environment - port policy: same program -> warn, different -> fail, and a **third case the rule implies**: a holder owned by another uid yields no pid from `ss`, nothing from `lsof`, and a `/proc/<pid>/fd` that cannot be walked, so its binary is unknowable and it can never be shown to be the same. That is `fail`. Levels are `outcome.LEVELS`, nothing added **The port check asks the right question.** `comm` and `exe` are `python3` for serena, for agentsview and for any stranger's server, so "what is this process" has no answer. The key is the **declared** program token from the item's own `command`, matched against basenames in the holder's `/proc/<pid>/cmdline`; the holder's argv[0] never enters the comparison. Five things were measured rather than assumed, and four of them were wrong the first time: 1. `subprocess.run(capture_output=True)` waits for the **pipes**, not the child. A server inheriting them held the recipe open for its whole life -- 25 s against 0 s redirected -- so `run_server` writes to a log file. Left alone this would have hung every run for the 20-minute recipe timeout and then called a healthy server a failure 2. `/proc/<pid>/cmdline` is **empty** immediately after `Popen`. Keyed on it, the pid-identity guard would have refused **every** stop, and silently, since a refusal is an `info`. The guard is `starttime`, field 22 of `/proc/<pid>/stat`, which is set at fork and readable across uids 3. a zombie keeps its `/proc` entry and its `starttime`, so the guard called a corpse ours and `stop_all` reported "survived SIGKILL". State `Z` now reads as gone 4. `ss -ltnH` prints **no Process column** -- the `-p` was missing, so every same-uid holder looked foreign and the `warn` branch the owner's rule turns on was unreachable 5. `start_new_session=True` is for the **stop**, not for survival. A server survives its parent by ordinary orphaning either way; without the flag `killpg` aims at a group that is not the server's. An earlier revision of the docstring claimed the opposite; removing the flag left the survival test green and turned the three stop tests red Guarantee, stated as such in the code: **stopped on normal and handled-error exit**. Not on SIGTERM (no handler) and not on SIGKILL. `PR_SET_PDEATHSIG` was measured to close the SIGKILL case and is named beside the limitation, with why it cannot be used at the spawning site. The start-up sweep is `docs/TODO.md` 4j, with what it costs to skip. 17 new tests, none mocked: real processes, real ports, a real detached start observed from a separate interpreter. `test_imports.py` gains `servers` in `BELOW` -- the wall test caught the new module before it was listed, which is now what that comment claims instead of the retired claim that the list was closed. tests/env_mgr: 502 passed, 2 skipped, 1 xfailed. Signed-off-by: yihou <yihou@amd.com>
…he file is Owner's ruling: everything except an agent's own assets/.claude/ tree is installed by declaring it in a recipe, and recipes come from three places. default env_mgr/default.env_recipe.yaml never declared package <staged>/assets/main.env_recipe.yaml never declared, auto-detected agent recipes: [...] on the agent spec declared No item carries a layer and none can: the path already says which layer a file is, and a field repeating it is a second writer of one fact. That is the reasoning that removed Item.layer, one level out. How a reader tells the default from a recipe you name: env_mgr/recipes/ is the namespace of things you NAME in recipes: [x]. The default is the one you never name, so it is not in that directory. Nothing marks it. THE PACKAGE LAYER ADMITS ONE SPELLING and that cost is deliberate. The agent layer takes every permutation because spec_loader finds it; env_mgr imports spec_loader NOWHERE (measured; a test enforces the partition), so it cannot reuse that machinery. Routing it through spec_loader would need the path on a field, and the only schema that could hold one is the task's -- giving every task its own recipe layer, a fourth layer arriving by accident. Two stated discovery rules beat one silent extra layer. A test pins the silence so it is visible in the suite and not only in a docstring. Layers CONCATENATE. A later layer adds; it does not override. That costs nothing because every installer gates on check before install. NOTHING DETECTS A VERSION CONFLICT BETWEEN LAYERS, and the docstring says why and what closing it would cost: detect_conflicts is scoped to one run() call and _run_recipe spawns one child per recipe file, so closing it means parsing all three in the parent -- the in-process coupling the subprocess design avoids. An autouse fixture points DEFAULT_RECIPE at nothing for the rest of test_agent_assets.py. Not tidiness: the shipped default runs on every install(), so without it every test there carries two extra report entries, and the second is "ok | uv already present" only on a machine that HAS uv. I made this suite host-dependent and this is the repair. 7 new tests. 4 go red on their own assertions against a tree with the layer prepending removed; the other 3 cannot -- two are negative assertions and one checks the shipped file rather than the wiring. Each says so in place. tests/env_mgr + spec_loader + agent + cli: 1080 passed, 2 skipped, 3 xfailed. Signed-off-by: yihou <yihou@amd.com>
…y alone "Start it once; a second declaration warns" needs no scope mechanism and no second declaration route. The registry already knows what is up, so which file declared the server is irrelevant: `already_running` looks the name up, and `install` reports `warn` and starts nothing. *Live*, not merely *present*. A crashed server must not make its own name unusable for the rest of the run, so the same `starttime` guard that decides a stop decides this. Two defects found by the tests for it: - **the registry check has to run before the port check.** A duplicate declaration finds its own first server holding the port, and the port check called it a stranger -- `fail` where the rule says `warn`. The first revision had the order the other way round and a comment claiming otherwise - **`exec myserver` declared the program `exec`.** Writing `command: exec ...` is the *good* form -- it saves a shell process -- and the token rule read `argv[0]` blindly, so the token matched nothing and a duplicate read as a stranger. The fixture in the new test is written that way, which is how it surfaced. A short closed list of wrappers (`exec env nohup setsid stdbuf time`) and `VAR=value` are skipped; it is not a parser, and a command the list does not cover degrades to `fail`, never to a false `warn` Also `docs/TODO.md` 4k: **installs run unconfined and §4 does not say so.** Measured while siting the registry -- `agent_assets._run_cmd` has no `preexec_fn` and no Landlock, so a recipe and every `run:` string under it execute with the supervisor's reach. That is arguably required, since an install writes outside every zone by definition; the gap is that §4 reads as though confinement is universal and names no exception. Recorded, not changed. 7 new tests. Red-checked: emptying the wrapper list reds three of the six parametrised cases, and the ordering defect was itself caught red before the fix. tests/env_mgr: 511 passed, 2 skipped, 1 xfailed. Signed-off-by: yihou <yihou@amd.com>
…t runner The two guards that keep `stop_all` from signalling a process this run does not own were asserted but **unprovable**: they used `os.getpid()` as the stand-in for a reused pid, so the mutation that makes them fail kills pytest. A guard proven by a test nobody can make go red is a guard nobody has checked -- and this is the one path in the feature where being wrong harms someone outside the run. Three serena listeners of unknown ownership were measured on this host the same day. A sacrificial child instead, with its real pid and a fabricated `starttime`: pid reuse simulated without waiting for the kernel to recycle a number, and nothing aimed at the runner. `start_new_session=True` on the sacrifice is a **safety requirement**, not a detail. `stop_all` calls `os.killpg`, which takes a process-group id; a child that is not a group leader shares the runner's group, so the guard-removed mutation would have aimed `killpg` at pytest after all. As its own leader it is the only thing in its group. `test_the_harness_can_actually_kill_the_sacrifice` is the positive control and the other two mean nothing without it. They assert a process is **not** signalled, which is also what they would assert if `stop_all` were broken, if the record never reached it, or if `killpg` aimed at nothing. With the correct `starttime` the same call kills the same child, so the aim is live. Mutation run, which could not be run before: `_still_ours` -> `True` turns all three red and pytest survives. Also `tests/cli`: the `cli/main.py` wiring had no executing test, and it is the line whose absence makes every server a leak while every unit test stays green. `test_a_run_names_a_server_registry_under_its_own_run_ root` stubs the three preconditions, raises from `permissions_enforced` -- the first call after the two lines under test -- and asserts the variable is set to `<run root>/servers.json`. Red-checked by deleting the two lines. tests/env_mgr + tests/cli: 677 passed, 2 skipped, 2 xfailed. `tests/cli` runs again now that `agent.schema.json` parses. Signed-off-by: yihou <yihou@amd.com>
… them Owner: "components不止agent_plugins，你这个命名也不合适，就叫addons". Second rename of this directory today, so the sweep used patterns with no word-boundary problem -- the last one left a test name intact because -w found no boundary inside "repositorys_components_directory". The move is not cosmetic. agent_plugins/ sat BESIDE env_mgr/, so it had no owning package to hang a package-data glob on and could not be shipped in a wheel at all -- 0 members, measured. Inside env_mgr/ it has one, and pyproject.toml now carries the entry. Proven by building the wheel and listing its members: 6 of 6 addon files present. THE FOURTH GLOB IS THE FINDING AND IT COST A BUILD TO SEE. With three patterns the wheel shipped 4 of 6 and looked right until they were counted: addons/*/.claude/**/* matches .claude/servers/x.py but NOT .claude/.mcp.json. The glob crosses a dot-DIRECTORY and will not match a dot-FILENAME at the leaf. Nothing in the pattern hints at it, so the pattern carries a comment that does. Half of temp/bugs/2026-09-04-two-declaration-routes-... is therefore closed, and the record now says so at the top rather than describing a fixed defect as open. The other half -- env_mgr/recipes/*.yaml, which the wheel still does not ship and which examples/env_checker depends on live -- is deliberately NOT fixed here. Bundling it behind a rename is how a defect stops being looked at. The wheel was built from a copy under /tmp/yihou/, not in place: the previous build left build/ and agent_sys_helper.egg-info/ in the repo, and two independent mechanisms then declined to delete them. Renamed with the directory: AGENT_PLUGINS_ROOT -> ADDONS_ROOT (one dirname shorter now), AGENT_SYS_AGENT_PLUGINS_ROOT -> AGENT_SYS_ADDONS_ROOT, _agent_plugin_trees -> _addon_trees, agent_plugin_grants -> addon_grants, and the validator's AGENT_PLUGINS_REL. The agent_plugins: DECLARATION KEY is untouched here on purpose -- it is being removed, not renamed a second time, and that is the next commit. tests/cli + tests/spec_loader + tests/env_mgr: 875 passed, 2 skipped, 2 xfailed. Signed-off-by: yihou <yihou@amd.com>
…is the one exception Two rules in spec 9.2, both about the same temptation -- reaching for Python because it is nearer than a recipe. Adding an MCP server or tool from code needs a justification that no declarative route works; running one inside the agent_sys process needs a justification that no separate process works. Neither forbids the thing; each says the reason must be written and must be that the other routes do not work. remote/tools.py is named as a standing exception with its closing condition, because it is delivered by injection into ClaudeAgentOptions and nothing is written to disk, so no installer can carry it. TODO 4l records the class behind a fixed defect: far_roots vs ctx.mapping, one question with two answer sources, failing by omission rather than by raising. Signed-off-by: yihou <yihou@amd.com>
scribe was stopped with the other four agents, so this section is hand-written rather than resurrecting an agent the owner had just stopped. Records the window the team was working in: four commits, the owner's exception ruling and two justification rules, two corrections of the lead's, and one finding that reopens agent_assets.py:287 -- the SDK's own docstring says the CLI loads project .mcp.json, and that comment says nothing reads it. Not resolved; needs an experiment rather than a fourth inference. Also records that the tree is left broken at 24 failed with a half-finished refactor, and why neither route back was taken. Signed-off-by: yihou <yihou@amd.com>
Consolidates this round's rulings and measurements into a single normative document instead of eight scattered ones. There are no levels: recipes come in three layers and the layer is where the file is; destination is derived from two questions, not declared; the transport decides whether the harness spawns a server or run_server does; Python-added capability and in-process servers each need a justification, with remote/tools.py the one named exception and its closing condition. Resolves by derivation what looked like a contradiction: .mcp.json is a project-scope filename and a zone config dir is user scope, so agent_assets.py:287 and the SDK's strict_mcp_config docstring are about different locations and both true. Signed-off-by: yihou <yihou@amd.com>
…s gone Narrow every claim that a component's `tools/*.tooldef.py` reaches an agent. `Prepared.tools` stays and is now scoped to its one member -- `remote/tools.py`, spec.provisioning.md 6's standing exception, injected as a live object with nothing written to disk. Anchor that section's "See ROADMAP.md" at the row that carries the closing condition. Signed-off-by: yihou <yihou@amd.com>
…'s exception The row itself is core-impl's, written before it was stopped and left uncommitted; I am committing their line and their sketch under my name because nobody else holds the file. Two things are mine. The opening clause said the in-process route "was deleted" flatly, which was true of the plan when it was written and was overtaken: only the COMPONENT-SUPPLIED half went. remote/tools.py stays, spec.provisioning.md 6 names it a standing exception, and the row as written told a future reader that a mechanism still running had been removed. And the closing condition is the run_server route this row already argues suffices -- not the thread sketch. Building the sketch is a different future and does not retire the exception. Spec 6 was already right; the row conflated them. Signed-off-by: yihou <yihou@amd.com>
… deleted
`spec.provisioning.md` §3-§5. There is one copy route — an agent's own
`assets/<name>.agent/.claude/` — and everything else is installed by declaring
it in a recipe. So the declaration key that reached `env_mgr/addons/` goes,
with the three other writers that had to agree for it to work:
`AgentSpec.agent_plugins`, the JSON-schema property,
`isolation/policy.py::addon_grants` and the exported `AGENT_SYS_ADDONS_ROOT`.
**Deleting the grant is 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. A recipe needs neither: installs run unconfined at prepare
step 6b and copy what they need into the zone.
Both `.mcp.json` files move to the one retained copy route,
`examples/env_checker/assets/env_probe.agent/.claude/`, byte for byte —
serena's keeps `--project ${AGENT_SYS_MY_WORKSPACE}` and `HOME: ${TMPDIR}`;
both are `"type": "stdio"`, which the harness spawns itself.
`env_mgr/addons/serena/` then holds nothing and is deleted; `envchk-baseline/`
keeps its server payload, which `assets/main.env_recipe.yaml` copies in.
Tests: 24 in `test_agent_assets.py` drove multi-tree behaviour through
`agent_plugins` as a vehicle. Fifteen are re-pointed at the one remaining tree;
four asserted a property that no longer exists (a cross-origin MCP collision,
two components' tooldefs) and are deleted with the unreachable warn branch in
`install`; two are rewritten to drive the `_place_tree` collision the way it
can still happen — a recipe writes into $CLAUDE_CONFIG_DIR and the agent's own
tree replaces it. Three new tests replace the deleted key's coverage: the copy
route is one, the three declaration writers are clean, and `install` exports no
path outside the zone.
pyproject: the addon package-data count is re-measured — 3 members on disk, 3 in
the wheel. The `.mcp.json` glob now matches nothing and is kept, with the
reason written down.
Signed-off-by: yihou <yihou@amd.com>
…tools spec.provisioning.md 6. A component that wants to offer a tool ships a server that runs on its own -- a tools/*.mcp.py, or a port-based one via run_server. Gone: TOOLDEF_SUFFIX, TOOLS_ATTR, IN_PROCESS_SERVER, _tooldefs, AgentMaterial.tools, Deployed.tools, and deployed.tools as a term in prepare's tools=. remote/tools.py STAYS and is untouched. It is 6's one standing exception -- injected into ClaudeAgentOptions as a live object with nothing written to disk, so no installer can carry it -- and Prepared.tools now carries it alone. The module docstring said WHY the supervisor imports third-party code. That prose became wrong rather than stale, so it is rewritten, not left: it now records the route as deleted and keeps the two measurements that are the argument for deleting it -- the supervisor holds the run's credentials, and an in-process tool read the supervisor's os.environ rather than the run's. New check, test_nothing_under_tools_is_ever_imported_into_the_supervisor, proved red three ways: restore the import loader -> the fixture raises at module scope; stop the fixture reaching the zone -> the presence assertion fires, not the inertness one; put AgentMaterial.tools back -> the hasattr assertion fires. The fixture keeps the .tooldef.py suffix on purpose, because a suffix nobody ever recognised could not go red. Signed-off-by: yihou <yihou@amd.com>
`importlib.util` had exactly one user, `_tooldefs`, which went in 8be9204. Found by `ruff --select F401` rather than by eye, and it is the only thing the removal left dead -- `sys`, `Sequence` and `_tool_files` all still serve the surviving `*.mcp.py` route. Signed-off-by: yihou <yihou@amd.com>
…ls are gone
Follows `spec.provisioning.md`, which supersedes the L1/L2/L3 vocabulary
entirely. Every L1/L2/L3 string in the package is removed, not renamed in place.
**Capability 6 is deleted, and the number is not reused.** It measured the
in-process `ToolDef` route, which §6 removed for component-supplied tools;
serena stays section 7 so that a reader counting 1-5 and 7 asks the right
question rather than inferring one was never there. `Capability.section` is now
a written field — the old index-plus-one convention cannot express a hole.
**What the package stopped proving, said rather than left to arithmetic**, in
`README.md`, `main.task/readme.md`, `env_probe.agent/README.md` and
`ACCEPTANCE.md`:
- that a `.claude/` tree this repository ships can be installed by declaration.
No such route exists.
- that sections 4 and 5 differ by who owns the declaring directory. Both entries
are now the agent's own `.mcp.json`; what still separates them — explicit
entry + recipe-installed payload vs location-as-declaration + copied — is
stated at each.
- `env_mgr`'s *load the placed copy, not the source* isolation property. That
was row 6b, the widest claim here, and `ACCEPTANCE.md` records that it must
come back if an in-process route does.
`Capability.level` becomes `installed_by` ∈ {recipe, copied}, and the reported
JSON key is renamed with it so that a report from an older brief fails on a
missing key rather than on a value mismatch that reads like the agent guessed.
The artefacts' `LEVEL` constants and the two MCP servers' payload field follow.
`check_capabilities_genuine` loses `agent_plugins_root` — the
`$AGENT_SYS_ADDONS_ROOT`-then-search-upwards guess for capability 4's artefact —
because the package recipe layer now copies that server into the zone;
`zone_config()` reads it from `<staged package>/../config/`, which is the same
measured sibling layout row 6b relied on.
serena.yaml: the agent recipe refers to it (`recipes: [serena]`, a bare name
resolving to `env_mgr/recipes/serena.yaml`), per the owner's ruling. Nothing in
`env_mgr/recipes/` moves. The loader's reservation on that line is answered in
place, together with the known gap that a bare name does not resolve from a
wheel install (spec §8).
Proved by driving both validators through their real `entry.sh` over a synthetic
zone: honest report GREEN; five mutations each RED on their own rule —
old `level` key, wrong route on section 4, a `tooldef` section returning,
deleting the recipe-placed server, and a forged section-4 token.
Signed-off-by: yihou <yihou@amd.com>
… of the lead's Second hand-written section: scribe is still stopped and a SendMessage would resurrect an agent the owner stopped. Records the ruling that changed how the round is run -- one spec so the answer has a place to live, rather than the lead alternating between deciding too much and asking everything -- and three corrections. The third is a new shape: a decision stated as if it were the artefact's current state, which is worse in a brief than in a claim because a brief gets executed rather than checked. Two teammate findings generalise past this round: read the content of a stopped agent's modified files before committing their path, and reason about how the observer will misattribute rather than only about whether your change is right. Signed-off-by: yihou <yihou@amd.com>
…children Two orphans left uncommitted by core-impl when the owner stopped it. Read for content before committing the paths, not just for whose hunks they were -- core2's rule, after an orphaned ROADMAP row was found asserting that a still running mechanism had been deleted. run_server's docstring: no shipped recipe uses it and that is not an omission. A stdio server is spawned by its client by definition -- the transport is the child's stdin and stdout -- so declaring serena here would be wrong rather than merely unnecessary. Both shipped addons are stdio, measured. test_run_server's reaper: WNOHANG waitpid, because a killed child stays a zombie until someone waits for it and CPython then prints 'subprocess N is still running' -- noise in a suite four agents share, and a claim that is not true. Signed-off-by: yihou <yihou@amd.com>
…NA_HOME
Two things, both verified against the artefact rather than taken from a list.
**`agent.schema.json`'s `assets` description still told a package author to
write `tools/*.tooldef.py`.** That is user-facing schema text, not a comment: a
stale comment misleads a maintainer, stale schema text tells someone to write a
file that does nothing. It now says so by name and points at the two routes that
do work — `tools/*.mcp.py` for stdio, a `run_server` recipe item for a port.
`envchk.Capability.surface`'s docstring justified itself by quoting a warning in
the brief about `mcp__env_mgr__…`, text that went with capability 6; the
argument is re-grounded on `envchk_baseline` vs `envchk_stdio`, which still
differ only in their server prefix.
Re-ran the grep rather than working the list: `env_mgr/addons/README.md` and
`env_mgr/docs/design.md` were already clean, and every other `ToolDef` hit is
`env_mgr/remote/tools.py`'s carrier, which stays (spec.provisioning.md §6).
**serena's `.mcp.json` entry: `HOME: ${TMPDIR}` becomes
`SERENA_HOME: ${TMPDIR}/.serena`.** Same directory, but it stops moving the
whole server process's home in order to relocate one application's state.
Verified before making the swap, because a narrower-looking variable that misses
a write path is worse than the blunt one. In serena 1.28.1 on this host:
`serena/config/serena_config.py:63` reads `SERENA_HOME`, and
`serena/ls_manager.py:64` passes that same directory as `solidlsp_dir` — so
`<dir>/language_servers/static`, where language servers are downloaded, is
covered rather than falling back to `solidlsp/settings.py:30`'s
`Path.home()/".solidlsp"` default. The `expanduser("~")` sites in
`solidlsp/language_servers/` are toolchain discovery for ruby/java/rust/haskell
and friends, none of them Python and none of them a write of serena state.
Then measured rather than left at a source read: started the real server with
only `SERENA_HOME` set and `HOME` untouched. The target captured
`serena_config.yml`, `logs/`, `memories/global/`, `prompt_templates/`,
`news_read.pkl` and `language_servers/static/PyrightServer`; `~/.serena` was
byte-identical across 210 entries before and after, and no `~/.solidlsp`
appeared.
Signed-off-by: yihou <yihou@amd.com>
A follow-up to `1d6a37ff`, which swapped serena's `.mcp.json` entry from
`HOME: ${TMPDIR}` to `SERENA_HOME: ${TMPDIR}/.serena` but did not say what the
old override had been doing.
I had described the change as a trade — a narrower write knob bought with a
wider read surface. **That was wrong, and the correction is the lead's.** `HOME`
is a lookup convention, not a permission: a process running as the operator can
read `/home/<operator>/...` by absolute path whichever way `$HOME` points, and
what bounds a read is the confinement `prepare` applies, which neither setting
touches. So `HOME=${TMPDIR}` never restricted anything — it redirected
`~`-relative **discovery**, which for serena is the `expanduser("~")` toolchain
lookups in `solidlsp/language_servers/` (rvm, .m2, .gradle, .cargo, .ghcup).
Dropping it restores those to the machine's real toolchains and `SERENA_HOME`
governs the writes for the first time: a strict improvement.
Recorded in the file rather than left in the thread, because the trade reading
is the plausible one and a later reader would arrive at it unaided.
Signed-off-by: yihou <yihou@amd.com>
…s of the lead's Third hand-written section; scribe is still stopped. The round's implementation is in: 2258 passed, tree clean, every ruling landed. The corrections are the content. Two were misattributions -- a teammate's name on diffs it did not write, and a stale read reported as a state. The third cost a teammate's time: I dispatched work that already existed, having acted on a report without checking the tree, which is the exact thing I had told both agents twice. Also records pkg2's species: a claim that is wrong and unwritten still costs something, because a wrong belief held privately is a file that has not been written yet, and it gets written when someone asks -- reading as considered rather than improvised. Signed-off-by: yihou <yihou@amd.com>
…a paragraph
`ACCEPTANCE.md`'s rows-6/6b block and `check_capabilities_genuine`'s *What it
cannot catch* both said the property row 6b owned "has no subject" and asked a
future reader to restore the row if an in-process route ever returned. **That was
a paragraph standing in for a check** — the species this package spends its
length arguing against, guarding the round's largest capability loss.
It is no longer true. `core2` shipped the guard in `8be9204e`:
agent_sys/tests/env_mgr/test_agent_assets.py
test_nothing_under_tools_is_ever_imported_into_the_supervisor
Both files now point at it and state what it does and does not buy. It places
two files under a `.claude/tools/` — one carrying the deleted route's own
`.tooldef.py` suffix — that **raise at module scope**, asserts both reached the
zone, and asserts nothing imported either. So it goes red the moment an
in-process route returns, and it fails by dying at the import rather than by a
later assertion.
Scoped honestly at both sites: the test is **stronger than row 6b while it
holds** — "nothing is imported" subsumes "which copy was imported" — and says
nothing at all about which copy the instant it stops. Whoever turns it green
again owes these two records a replacement, and the failing test is what tells
them.
**Named by path plus symbol rather than described**, and both pointers were
resolved before committing. A prose reference to a test rots silently; a path
either opens or does not. This round was bitten twice by references to things
that no longer existed — `ROADMAP.md`'s deleted-mechanism claim and
`Capability.surface`'s docstring quoting text that had been removed.
Signed-off-by: yihou <yihou@amd.com>
…roves itself env_checker lost "load the placed copy, not the source" when the in-process route went (8be9204). pkg2 recorded the loss in ACCEPTANCE.md and in the genuine validator's "what it cannot catch", and named the weakness itself: both records are prose, and prose does not go red when the route returns. This is the wide form. The sibling test pins the deleted suffix; this pins the rule that suffix was one instance of, so a future route importing hooks/*.py or a file at the tree root fails here even though the narrow test stays green. Two detectors, because either alone has a hole. Every fixture raises at module scope, so an EXECUTED member is a RuntimeError out of install even if the module is popped afterwards; the sys.modules diff catches an import that SUCCEEDS, which no amount of raising would reveal. Both source tree and placed copy are checked, because importing either is the defect env_checker used to catch. The detector proves itself on EVERY run, not only under a hand mutation: the test imports a placed member on purpose and requires the same detector to flag it. A negative assertion about a deleted mechanism is precisely the case that passes when its subject is absent, so that control is not left to whoever edits this file next. Proved red four ways, each firing its own assertion: import a raising member -> RuntimeError; import only the benign member -> caught by the sys.modules diff alone; keep a fixture out of the zone -> the PRESENCE assertion fires, not the inertness one; blind the detector -> the self-test fails. Signed-off-by: yihou <yihou@amd.com>
`6515c856` added `test_no_member_of_a_claude_tree_is_ever_imported_into_the_supervisor` beside the narrow one, and both of this package's records of row 6b's loss named only the narrow test — so they understated what now exists. The narrow test pins the deleted route's `.tooldef.py` suffix. The wide one pins the rule that suffix was one instance of: a component's `.claude/` tree is data this process places, never code it runs. It covers every place a component can put a file — the tree root, `tools/`, `hooks/`, `skills/<name>/` — so a future route importing `hooks/*.py` fails there while the narrow test stays green. Wording from `core2`, who wrote the test, with two corrections the lead ruled after I read it. Both narrow a claim rather than widen one: 1. **"its detector is proved on every run" named the wrong scope.** What self-proves is the `sys.modules` diff specifically: the test imports a planted benign member at the end and requires the diff to return exactly it. The raise-at-module-scope detector is sound by construction and is not exercised. Both records now say which. 2. **The both-roots property belongs to the diff, not to the test at large.** Covering the staged source and the placed copy is what the `sys.modules` comparison does, and it is the closest thing to row 6b that now exists — attributing it precisely is what makes the "still not row 6b" sentence land. The limitation `core2` refused to soften is kept at both sites and is the point of the edit: these tests assert nothing was imported, which is strictly stronger than row 6b while it holds and says nothing at all about *which copy* the instant it stops. Whoever turns them green again owes both records a replacement, and the failure is what tells them. Four path+symbol pointers now across the two files, all four resolved before committing — file exists and `def <symbol>(` present. Signed-off-by: yihou <yihou@amd.com>
…class
`pyproject.toml`'s comment above `[tool.setuptools.package-data]` describes this
failure mode exactly -- "`packages.find` seeing the package does not ship the
`.json` files inside it" -- and was written when `spec_loader/schemas` was
fixed. `addons/` was the second instance, fixed earlier today. **These are the
third and fourth, and they were sitting under the comment that explains them.**
What found them was not reading the comment again. It was
`find env_mgr -type f ! -name '*.py'` -- enumerating the class's population
instead of the instance in front of me. Fixing an instance is not sweeping a
class, and the sweep is one command.
env_mgr = ["default.env_recipe.yaml", "recipes/*.yaml", ...]
`recipes/*.yaml` is what a bare `recipes: [<name>]` resolves against, so without
it that declaration raised from a wheel and resolved from a checkout --
`examples/env_checker` declares `recipes: [serena]`.
**`default.env_recipe.yaml` is the worse of the two and is why the sweep was
worth running.** It is the DEFAULT recipe layer, it runs on every install, and
`_recipe_paths` gates it on `os.path.isfile`. Nothing declares it, so its
absence is "simply absence" by design: from a wheel the whole default layer
would have done nothing, reported nothing and named no cause. The bug record
being closed here explicitly claimed the opposite -- "That the failure is
silent. **It is not**" -- which was true of the two routes it examined and false
of the one it did not. The correction is in the record.
Counted, then resolved. The wheel carries 3 of 3 members; it was then installed
into a clean venv and `agent_assets._recipe_paths({"recipes": ["serena"]})`
called against the install, returning both layers, with `serena.yaml` parsing to
3 items. Counting members proves the files are in the archive; calling the
resolver proves the route works. The first attempt imported the checkout rather
than the venv because the working directory put it on `sys.path` -- caught by an
assertion on `env_mgr.__file__`, which is the only thing that could tell them
apart.
`recipes/README.md` is deliberately not shipped: nothing reads it at run time.
Recorded so the next person counting members does not read it as an omission.
Signed-off-by: yihou <yihou@amd.com>
Fourth hand-written section; scribe is still stopped. Three corrections of the lead's. The first is the worst shape of the round: I stopped a teammate citing a line number as proof its work was redundant, and the line number existed only because the new work had landed above it -- the displacement was inside the number I quoted. Also records the design ruled this window (a recipe reference carries its root, no bare form and no fallback) and pkg2 leaving out a sentence that was true, because a limitation stated twice from opposite sides is not somewhere to add a hedge. Signed-off-by: yihou <yihou@amd.com>
`recipes: [serena]` did not say which root it meant. `_recipe_paths` built a
package-relative candidate and a shipped-directory candidate and took the first
that **existed**, so reading the declaration required knowing the resolution
order, and the root was selected by what happened to be on disk.
A reference is now `<scheme>:<ref>` and there is no bare form and no fallback:
agent_sys:<name> -> env_mgr/recipes/<name>.yaml
package:<relpath> -> <staged package>/<relpath>
anything else -> PrepareRefused naming both spellings
`package:` declared-and-absent is an error -- `material.py`'s existing rule,
reaching this key for the first time -- rather than a silent demotion to the
other root. A bare name gets a dated migration guard in the shape of
`recipe.py`'s `_LAYER_REMOVED`, kept separate from the unknown-scheme branch
because "names no root" and "is not a recipe root" send a reader to different
questions; the constant says when it can go. `agent_sys:` and not `env_mgr:`
because the root is a repository-level fact and naming the module would have
been wrong the day the directory moved, which it did today.
`agent_sys:<name>` now refuses a separator instead of calling `os.path.basename`
on it. Silently turning `a/b` into `b` is the same species this change removes.
The old docstring defence is kept rather than deleted -- *"a package cannot
shadow a shipped recipe by accident, because its own path has to resolve
first"*. It is an argument that the **precedence** is safe, and it is correct.
It is not the objection: the reference was illegible, not dangerous.
## One claim I wrote, then measured, then withdrew
I justified this in six places with "a typo in a package path silently ran a
different recipe". **That is false**, and the red-check is what found it: with
the fallback reintroduced, every one of the eleven refusal rows stayed green.
Driving the old candidate logic directly: the shipped candidate was
`<shipped>/<basename>.yaml`, so `package/recipes/tools.yaml` fell back to
`tools.yaml.yaml` and **refused**. A mistyped path was loud. What was silent is
a *shadowed name*: a staged package carrying a file named exactly `serena` won
over the shipped `serena.yaml`, same declaration, different file, reported as
neither. Corrected in the docstring, the guard message, the JSON schema,
`recipes/README.md`, `recipes/serena.yaml` and `check.yaml`.
The test was rewritten to the measured defect and is the only one that goes red
when the old precedence is faithfully restored -- the other eleven name things
absent from both roots and so cannot tell a refusal from a fallback that also
missed.
Red-checked by removing each guard in turn: no bare-name guard (2 rows red),
any scheme accepted (2 red), basename normalisation restored (2 red), old
precedence restored (1 red, the shadowing test). Migrated the one live usage,
`examples/env_checker` -> `recipes: [agent_sys:serena]`, and the prose in
sixteen files that spelled the old form.
Signed-off-by: yihou <yihou@amd.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Please include a brief summary of the changes, relevant motivation and context.
Fixes # (issue)
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: