Skip to content

emrg: the shell tools stop advising a workdir the daemon replaces - #2701

Closed
pm25coder wants to merge 1 commit into
masterfrom
fix/the-shell-description-does-not-offer-what-the-daemon-replaces
Closed

pm25coder wants to merge 1 commit into
masterfrom
fix/the-shell-description-does-not-offer-what-the-daemon-replaces

Conversation

@pm25coder

Copy link
Copy Markdown
Collaborator

Closes #2698

What changed

Both shell descriptions advised a parameter that cannot work. They said:

Each call is a fresh process — … so pass workdir instead of using cd.

The daemon writes the session's cwd into a shell call's workdir unconditionally
(EmrgServer._inject_tool_arguments), and when a sandbox tier is configured it also writes
workspace, which both executors read first — so a directory the caller names is discarded twice over
and 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 now
    names the mechanism that works (cd inside the command) and who decides the directory (the daemon),
    in the same words in both dialects.
  • The same two files' workdir field now says the daemon supplies the value and replaces whatever the
    call 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 that workdir was a
    declared property, a reading that cannot fail while the parameter exists, whatever the daemon does
    with the value. It now drives _inject_tool_arguments with a session stub and reads the pair as one
    fact: 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.

glob and grep are untouched: their workdir/path decides a preference rather than a boundary, and
tests/test_shell_tool_mount.py still pins that as a separate class.

Readings

Measured on this host, with emrg.__file__ printed inside the checkout:

  • the description file's own suite: 18 passed (it was 16 — one guard became two, per dialect);
  • tests/test_shell_tool_mount.py tests/test_pwsh_tool_v2.py tests/test_tool_argument_domains.py:
    120 passed, 1 skipped;
  • the whole suite: 5628 passed, 518 skipped in 1214.70s. That total is this tree's, not this PR's:
    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:

  • the retired advice put back into the bash description alone ⇒ 2 failed, 16 passed, and both
    failures are the [bash] parametrization while [pwsh] stayed green — the reading is per dialect,
    not per file.
  • the injection made conditional again (and "workdir" not in args, the pre-D1 form) ⇒ 4
    failed
    : 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_model and
    test_the_pinned_class_is_the_tools_that_run_a_command, which is how the arm shows the mutation
    landed.

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 dropping workdir is real — and it is
deliberately not taken here.

@pm25coder

Copy link
Copy Markdown
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 pass workdir`` is absent, which also catches a re-wording that keeps the invitation, while #2700 pins the retired clause exactly.

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.

emrg: the shell tools tell the model to pass workdir, which the daemon replaces on every call

1 participant