fix(tools): validate per-call workdir and preserve resolved TERMINAL_CWD#4731
fix(tools): validate per-call workdir and preserve resolved TERMINAL_CWD#4731HenkDz wants to merge 1 commit into
Conversation
Fixes NousResearch#4669 and NousResearch#4672. - gateway/run.py: skip re-bridging cwd config key when TERMINAL_CWD is already an absolute path (cli.py resolved it from '.'). Without this, gateway overwrites the resolved path with raw '.' and falls back to Path.home(). - tools/terminal_tool.py: validate workdir before passing to env.execute(). Malformed paths (e.g. '/workspace/..???') are silently passed through to 'docker exec -w', causing command failures that look like Docker is ignoring terminal.cwd. Now falls back to session cwd with a warning.
|
Thanks for the thorough diagnosis, @HenkDz! Both bugs this PR targets have since been fixed on Bug #4672 — TERMINAL_CWD clobber in gateway/run.py Fixed in commit Bug #4669 — invalid workdir forwarded to docker exec Fixed in commit Note: the behavior on main is to block and error on an invalid workdir rather than fall back to session cwd as this PR proposed. If you have a use-case where silent fallback is preferable to an explicit error, feel free to open a new PR or comment on the linked issues. |
Fixes #4669 and #4672.
Summary
Two related bugs where
terminal.cwdgets silently overridden by downstream code that doesn't validate/resolve the path properly.#4672 — gateway/run.py clobbers resolved TERMINAL_CWD
cli.pyresolvesterminal.cwd: "."toos.getcwd()at import time, butgateway/run.pyre-bridges the config and setsTERMINAL_CWD = "."(raw string) when imported as a plugin. This overwrites the absolute path, and later code falls back toPath.home().Fix: Skip the
cwdkey in gateway's terminal env bridge whenTERMINAL_CWDis already an absolute path.#4669 — terminal tool passes invalid workdir to Docker exec
When a per-call
workdiris provided but invalid (e.g./workspace/..???), it gets passed straight todocker exec -w <invalid-path>, causing silent command failures. This makes it look like Docker is ignoringterminal.cwd, but the real problem is that invalid per-callworkdiroverrides the session cwd without validation.Fix: Validate
workdirbefore passing toenv.execute(). If the path is malformed or relative (ambiguous in container backends), fall back to the session cwd with a warning log.Changes
gateway/run.py— skip clobbering already-resolved TERMINAL_CWDtools/terminal_tool.py— validate workdir, fall back to session cwd if invalid