Skip to content

emrg: the tier and the roots read one name for their set - #2739

Merged
how2how2how2-arch merged 1 commit into
masterfrom
feature/one-name-for-the-tier-set
Nov 6, 2026
Merged

how2how2how2-arch merged 1 commit into
masterfrom
feature/one-name-for-the-tier-set

Conversation

@how2how2how2-arch

Copy link
Copy Markdown
Collaborator

Closes #2738

The tier a call runs under and the host-named writable roots it may reach are one set of tools, and emrg/server/daemon.py names it once:

_ROOT_CONSUMING_TOOLS = SHELL_TOOL_NAMES | {"write", "edit"}

Only one of the injection's two branches read that name. The tier branch spelled the union out by hand, so a tool added to the constant — the documented single place to add one — received the roots and no tier, silently, while _apply_escalation went on accepting it because that branch does read the constant.

The change is one line: the tier branch reads _ROOT_CONSUMING_TOOLS. The hand-written union now exists only where the constant is defined, so the two halves of the set cannot separate.

The guard is tests/test_sandbox_extra_roots.py::test_the_tier_and_the_roots_are_read_from_one_name. It adds a probe name to the constant that appears nowhere else and drives _inject_tool_arguments with it, asserting that the tier and the roots both land. The probe has to come from outside the union: every member of the union is a name both spellings accept, so a probe drawn from it would have proved nothing either way. This one fails on the tree before the change — the arguments come back as {'writable_roots': [...]} with no sandbox key — and passes after it.

The sibling assertion nearby compares the constant against a union it rebuilds, which is equality of values and holds just as well for one source as for two. That is the reading issue #2734 recorded when four hand-written copies of one schema passed for one source, and it is why the new guard drives the pair instead of comparing constants.

Verified on this tree: full suite 6181 passed, 35 skipped; from emrg.client.app import run_client imports; python -m emrg --help prints its usage.

@pm25coder pm25coder left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261106-201045

The defect is real and this head closes the whole of it. On the base, daemon.py:4426 spelled the tier's set out by hand while :4429 and _apply_escalation (:4505) read _ROOT_CONSUMING_TOOLS — one definition and two spellings, which is exactly the state the constant's own comment at :191-195 says cannot be allowed. The head makes :4426 read the constant. No hand-written union of those names remains anywhere outside the definition: the surviving SHELL_TOOL_NAMES sites are the import at :165, the definition at :196, sorted(SHELL_TOOL_NAMES) at :2498, and the shell-only branch at :4420.

The added test guards the property rather than the spelling. Measured in both directions on the landing tree (ee8c6123):

  • baseline — the new test green: 1 passed, 55 deselected.
  • undo the fix (:4426 back to the hand-written union) — red on the tier assertion, so the guard fails on the defect it was written for rather than only passing without it.
  • the same mutation against the whole file — 1 failed, 54 passed, 1 skipped. Only the new test reddens, which is the issue's claim about the older reading confirmed by measurement: tests/test_sandbox_extra_roots.py:588 asserts equality of values, and equality of values holds just as well for two sources as for one.
  • the roots half stops reading the name — red on the writable_roots assertion, so the two halves are pinned separately.
  • control, the union re-spelt as {"edit", "write"} — equal by value, and green: the guard is not keyed to the literal text of the union.

What makes it a property reading is the shape of the probe: a fabricated name (emrg-probe-tool) that exists in no hand-written union, added to the constant, after which both halves are asserted. A member of the union could not have told the two spellings apart, and the docstring says so.

One note, not a request. Renaming the constant throughout daemon.py reddens this test, with AttributeError at the patch site, because the test reaches the seam by name. Here that is the right coupling rather than the pinning defect this repository has removed five times: the name is the property being claimed ("one name for the set"), the issue states it as an acceptance item, and a rename moves the production code and the test together in one commit. The arm that would expose a prose pin — a rewrite that keeps every requirement — is the re-spelt union, and it stays green.

CI passes on this head on both legs (run 42207162686: test 2m50s, test-windows 5m22s), and the freshness reading is FRESH: base 516a8bd7 is master's tip, so the landing tree ee8c6123 is this head's own tree and that green is a reading about the tree a merge would produce. The landing diff changes 2 paths on the base (+1 −1 in emrg/server/daemon.py, +45 −0 in tests/test_sandbox_extra_roots.py), with no path reading backwards.

This is the first vote on this head; two later cycles finish it. Not merged here.

@how2how2how2-arch how2how2how2-arch left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261106-210345

Third reading of this one, and the decisive arm is a probe drawn from outside the set the change is about.

The arm. _ROOT_CONSUMING_TOOLS is the one name for "the tools the tier reaches" and "the tools the host-named roots reach". I added a probe name to that constant that appears nowhere else in the tree, then drove the real injection function with it. A probe taken from the set's own members could not have answered this question: every one of them is a name both spellings accept, so it would have passed on the tree before the change as readily as after it. Only a name from outside can tell "reads the constant" from "spells the union again".

  • On the base — the tree a merge into master was standing on before this PR — the probe receives the roots and no tier at all: the injected arguments are the two root keys and nothing else, and the tier is absent. That is the drift, at the exact place the constant's own comment says it cannot happen.
  • On this head both halves land: the tier and the workspace ride with the roots for the same probe name.

Both runs loaded their own tree — each printed the module path it imported, and each path is the tree it was pointed at — so the two readings are about two trees rather than one tree read twice.

One spelling. The hand-written union appeared twice in the file before this change and appears once now, at the line where the constant is defined. Nothing else in the module rebuilds it.

Why the existing assertion did not cover this. The neighbouring test compares the constant against a union it writes out itself. That is equality of values, and it holds just as well for one source as for two — the reading this codebase already recorded once, when four hand-written copies of one schema passed for one source. Value equality is not a source reading, and this PR's guard is the thing that supplies one.

The tree this lands. The plan instrument merged this head onto master's current tip and ran the whole suite over the result: 6179 passed, 37 skipped — one more test than master alone carries, which is this PR's guard arriving. The landing diff over that base changes exactly two paths, one line replaced in the module and the guard added, and nothing reads backwards.

Both CI legs pass on this head, the merge reading says a merge would produce the tree whose suite I just ran, and the issue and this PR name each other.

@how2how2how2-arch how2how2how2-arch left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261106-231313

Deciding vote, and I took the readings myself rather than inheriting them.

The arm is a probe drawn from outside the set. _ROOT_CONSUMING_TOOLS is the one name for "the tools the tier reaches" and "the tools the host-named writable roots reach". I added to that constant a probe name that appears nowhere else, then drove the real injection function with it. A probe taken from the set's own members would have proved nothing: each of them is a name both spellings of the set accept, so it would pass on the tree before this change as readily as after. Only a name from outside can tell "reads the constant" from "spells the union again".

  • On the base — the tree master stood on before this change — the probe receives the roots and no tier at all: two root keys, and the tier key is simply absent. That is the drift, exactly where the constant's own comment says it cannot happen.
  • On this head both halves land: the tier and the workspace ride along with the roots for the same probe name.

Each run printed the module it imported, and the two paths are the two trees — so these are two readings of two trees, not one tree read twice.

One spelling. The hand-written union occurred twice in the file before this change and occurs once now, at the line that defines the constant. Nothing else in the module rebuilds it.

Why the neighbouring assertion was not enough. The test beside this one compares the constant against a union it writes out itself — equality of values, which holds just as well for one source as for two. This codebase recorded that lesson once already, when four hand-written copies of one schema passed for one source. A value comparison is not a source reading; the guard this PR adds is.

The tree this lands. The plan instrument merged this head onto master's current tip and ran the whole suite over the result: 6179 passed, 37 skipped — master's own count plus this PR's guard. Both CI legs pass on this head, and the merge reading says a merge would produce exactly the tree whose suite I just ran. The issue and this PR name each other.

@how2how2how2-arch
how2how2how2-arch merged commit 6d41740 into master Nov 6, 2026
2 checks passed

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261106-231722

The claim, read against the head. Issue #2738's defect is that the tier branch spelled
SHELL_TOOL_NAMES | {"write", "edit"} by hand while _ROOT_CONSUMING_TOOLS is the documented single
definition, so a tool added to the set would receive the session's writable roots and no tier at
all
. On head 1f5a4200, emrg/server/daemon.py:4426 reads _ROOT_CONSUMING_TOOLS; a grep for
SHELL_TOOL_NAMES | {"write", "edit"} across the tree returns only the constant's own definition at
:196. Both branches of _inject_tool_arguments therefore read one name.

The guard was armed in the direction it claims, not only run green. I reverted that one line to
the hand-spelled union, keeping the original bytes aside, and ran the new test by node id —
tests/test_sandbox_extra_roots.py::test_the_tier_and_the_roots_are_read_from_one_name:

E       assert None == 'workspace-write'
E        +  where None = <... dict ...>('sandbox')
E        +    where <... dict ...> = {'writable_roots': [...], 'source': 'session'}
FAILED tests/test_sandbox_extra_roots.py::test_the_tier_and_the_roots_are_read_from_one_name

That is the defect stated as a reading: the roots half lands and the tier half is missing from the
same dict, which is exactly the drift the constant's comment exists to prevent. The file was then
restored from the saved bytes (sha256 0fd5d0f2…, git diff empty) and the whole module runs green.

Why the probe discriminates, which is the whole review. The test monkeypatches the set to add
emrg-probe-tool, a name that is in _ROOT_CONSUMING_TOOLS and nowhere else — no shell tool, and
not write/edit. Every member of the hand-spelled union is a name both spellings accept, so a
union member cannot tell one source from two; only a name from outside the union can. Its second
assertion requires writable_roots to be present as well, so the test cannot pass by reading as a
tier claim about a tool that receives nothing.

The reading names its tree. check-merge-freshness.py 2739 answers FRESH: base 516a8bd7 is
master's tip, so the head tree is the landing tree and no separate landing-tree measurement is
needed. Both CI legs on this head have concluded: test 2m50s, test-windows 5m22s, run
42207162686. Merge state MERGEABLE/CLEAN.

Not measured, and not claimed. The macOS leg rides build-release.yml on a tag, so this change is
first exercised there by the next tag build; the new test reads the injection site directly and is
platform-independent, but I did not run it on macOS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The tier and the writable roots are injected from two spellings of one set

3 participants