Repository navigation
Conversation
Collaborator
Author
|
Folded into #2700: same issue, same three files, same reading, opened 14 minutes earlier (14:11:31Z against 14:25:29Z here). One issue is finished by one PR, so this head is closed rather than left standing beside it, and a later cycle reviews #2700 on its own reading. The two heads differ only in wording and in how wide the negative reading is: this one asserts that |
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
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.
Closes #2698
What changed
Both shell descriptions advised a parameter that cannot work. They said:
The daemon writes the session's cwd into a shell call's
workdirunconditionally(
EmrgServer._inject_tool_arguments), and when a sandbox tier is configured it also writesworkspace, which both executors read first — so a directory the caller names is discarded twice overand the command runs in the session's working directory. That injection has to stay: honouring a
model-supplied directory let the model name the root it was trusted in (that method's docstring,
design §2.4). The sentence had to change, not the kernel.
emrg/tools/bash_tool_v2.py,emrg/tools/pwsh_tool_v2.py— the shared fresh-process sentence nownames the mechanism that works (
cdinside the command) and who decides the directory (the daemon),in the same words in both dialects.
workdirfield now says the daemon supplies the value and replaces whatever thecall gives, so a reader who sees the field is not left thinking it moves the command.
tests/test_shell_tool_descriptions.py— the guard standing here asserted only thatworkdirwas adeclared property, a reading that cannot fail while the parameter exists, whatever the daemon does
with the value. It now drives
_inject_tool_argumentswith a session stub and reads the pair as onefact: the caller's value is replaced, and no description of that tool may then offer the directory to
the caller. A second guard reads the field's own sentence. The module docstring records the case,
since that file exists to read each promise against the half that decides it.
globandgrepare untouched: theirworkdir/pathdecides a preference rather than a boundary, andtests/test_shell_tool_mount.pystill pins that as a separate class.Readings
Measured on this host, with
emrg.__file__printed inside the checkout:tests/test_shell_tool_mount.py tests/test_pwsh_tool_v2.py tests/test_tool_argument_domains.py:120 passed, 1 skipped;
master has gained tests since the last figure recorded anywhere here, and the only collected change
this PR makes is +2, in the file named above.
Two arms, each restored byte-identically and checked by sha256 after the restore:
failures are the
[bash]parametrization while[pwsh]stayed green — the reading is per dialect,not per file.
and "workdir" not in args, the pre-D1 form) ⇒ 4failed: the new guard on both dialects — its precondition reports that the daemon no longer replaces
the value, so the negative reading would be about nothing — plus
test_the_session_cwd_cannot_be_named_by_the_modelandtest_the_pinned_class_is_the_tools_that_run_a_command, which is how the arm shows the mutationlanded.
What this leaves
The parameter is still declared, and both executors still honour it when called directly, so the
schema's sentence is where a caller learns it is not theirs to set. Removing the declaration outright
would be a design change worth arguing on its own — injected keys (
workspace,sandbox,_session_id) are not declared to the model, so the case for droppingworkdiris real — and it isdeliberately not taken here.