Skip to content

Commit cdfe707

Browse files
authored
fix(cron): redirect file-edit tools to a job's workdir, not just git (#585)
A cron AGENT job's `workdir` only set `TERMINAL_CWD`, which the terminal/git tools read live — so `git` honoured the workdir, but the file-edit tools did not. `tools/file_tools._resolve_base_dir` resolves relative paths against the task's *live* terminal-env cwd (priority #1), which in a long-lived gateway/daemon is the shared "default" env still anchored at the runtime checkout. That #1 cwd shadowed `TERMINAL_CWD` (#3): `write_file`/`patch` landed in the runtime mirror while git committed in the workdir — a split-brain that dirtied the runtime checkout the evolution pipeline mirrors. Fix (cron/scheduler.py::_run_job_impl workdir setup + finally only): - When a live "default" terminal env exists at job start, point THAT env's `.cwd` at the workdir (under `_env_lock`), capturing the prior cwd. That one object backs both `_resolve_base_dir` (#1) and the shell-backed `ShellFileOperations`, so both file-tool paths follow the workdir. The prior cwd (incl. `None`) is restored under the lock in `finally`, mirroring the existing `TERMINAL_CWD` save/restore — no leak into a later job. - Cold start (no live env yet): the file tools already fall through to `$TERMINAL_CWD` (#3) = workdir, and a terminal command run during the job lazily creates the shared env seeded from it. On cleanup that env's cwd is cleared (non-destructively — no os.chdir / teardown) and the file-ops cache dropped, so it cannot leak the workdir into a later workdir-less job (the same leak `TERMINAL_CWD` already had for that env's git). NOT touched: the `no_agent` script path (real per-job `os.chdir`), workdir-less jobs (verified untouched), and the file/terminal tools themselves. Workdir jobs are serialized by `tick()`, so mutating the shared in-process env cwd has the same (already-accepted) blast radius as the existing process-global `TERMINAL_CWD` mutation. The remaining shared-"default"-env concurrency between a sequential workdir job and concurrent parallel workdir-less jobs is a pre-existing architectural property (true isolation needs task-scoped env keys) and is out of scope here. TDD: new tests reproduce the split (file base dir resolves to the runtime checkout while git follows the workdir) — failing pre-fix, green post-fix — plus regressions that a workdir-less job is unaffected, that a workdir job does not leak into a following workdir-less job, and that a lazily-created env is neutralised on cleanup. Cross-reviewed by two non-Claude advisors (Gemini, Kimi).
1 parent ffff7e6 commit cdfe707

2 files changed

Lines changed: 415 additions & 0 deletions

File tree

cron/scheduler.py

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2176,6 +2176,93 @@ def strip_reasoning_for_delivery(text: str) -> str:
21762176
).strip()
21772177

21782178

2179+
# Tool-call task id that the top-level cron agent's file/terminal tools collapse
2180+
# to (see tools/terminal_tool._resolve_container_task_id). The split-brain lives
2181+
# on this shared env: its live cwd is file-tool priority #1 and shadows
2182+
# $TERMINAL_CWD (#3) for a workdir job.
2183+
_CRON_FILE_TOOL_TASK_ID = "default"
2184+
2185+
2186+
def _seed_workdir_file_tools(workdir: str):
2187+
"""Redirect the FILE-EDIT tools at *workdir* too — not just the terminal.
2188+
2189+
``_run_job_impl`` already points ``TERMINAL_CWD`` at the job's workdir, which
2190+
the terminal/git tools read live. The file-edit tools (write_file/patch/...)
2191+
resolve relative paths through ``tools/file_tools._resolve_base_dir``, whose
2192+
top priority (#1) is the task's *live* terminal-env cwd. In a long-lived
2193+
gateway/daemon the shared ``"default"`` env still points at the runtime
2194+
checkout, so it shadows ``TERMINAL_CWD`` (#3): git honours the workdir while
2195+
``write_file`` lands in the runtime mirror and dirties it (split-brain).
2196+
2197+
The fix is surgical: when such a live env exists, point ITS cwd at the
2198+
workdir. That single object is what both ``_resolve_base_dir`` (#1) and the
2199+
shell-backed ``ShellFileOperations`` read, so both file-tool paths follow the
2200+
workdir. When no live env exists yet the file tools already fall through to
2201+
``$TERMINAL_CWD`` (#3) — but a terminal command run *during* the job lazily
2202+
creates the shared env seeded from ``$TERMINAL_CWD`` = workdir; left alone it
2203+
would persist and leak the workdir into a later (workdir-less) job (the same
2204+
way ``TERMINAL_CWD`` already leaks to such an env's git today). The returned
2205+
token records which case applied so :func:`_restore_workdir_file_tools` can
2206+
undo exactly what happened.
2207+
2208+
Returns an opaque restore token, or ``None`` if the terminal layer is
2209+
unavailable (fail-open: the job still runs with ``TERMINAL_CWD`` set).
2210+
"""
2211+
try:
2212+
from tools import terminal_tool as _tt
2213+
except Exception:
2214+
return None
2215+
key = _CRON_FILE_TOOL_TASK_ID
2216+
with _tt._env_lock:
2217+
env = _tt._active_environments.get(key)
2218+
if env is not None and hasattr(env, "cwd"):
2219+
prior_cwd = env.cwd
2220+
env.cwd = workdir
2221+
return ("existing", env, prior_cwd)
2222+
return ("absent", None, None)
2223+
2224+
2225+
def _restore_workdir_file_tools(token) -> None:
2226+
"""Undo :func:`_seed_workdir_file_tools`.
2227+
2228+
``existing``: restore the env's exact pre-job cwd (including ``None``) under
2229+
the env lock, mirroring the ``TERMINAL_CWD`` save/restore — the shared
2230+
``"default"`` env returns to precisely its prior state, no leak.
2231+
2232+
``absent``: neutralise any ``"default"`` env that was lazily created during
2233+
the job so its workdir cwd does not leak into a later, possibly
2234+
workdir-less, job. Non-destructive: only the ``cwd`` attribute is cleared
2235+
(no ``os.chdir``, no env teardown — the subprocess/container stays alive)
2236+
and the file-ops cache is dropped so a rebuilt ``ShellFileOperations``
2237+
re-reads the now-restored cwd. With a cleared cwd the file tools fall back
2238+
through ``$TERMINAL_CWD`` (restored) to the process cwd, exactly as a
2239+
workdir-less job expects.
2240+
"""
2241+
if not token:
2242+
return
2243+
kind, env, prior_cwd = token
2244+
try:
2245+
from tools import terminal_tool as _tt
2246+
2247+
if kind == "existing":
2248+
with _tt._env_lock:
2249+
env.cwd = prior_cwd
2250+
return
2251+
key = _CRON_FILE_TOOL_TASK_ID
2252+
with _tt._env_lock:
2253+
lazy_env = _tt._active_environments.get(key)
2254+
if lazy_env is not None and hasattr(lazy_env, "cwd"):
2255+
lazy_env.cwd = None
2256+
try:
2257+
from tools.file_tools import clear_file_ops_cache
2258+
2259+
clear_file_ops_cache(key)
2260+
except Exception:
2261+
pass
2262+
except Exception:
2263+
pass
2264+
2265+
21792266
def run_job(job: dict) -> tuple[bool, str, str, Optional[str]]:
21802267
"""Execute a single cron job."""
21812268
job_id = job["id"]
@@ -2458,9 +2545,17 @@ def _run_job_impl(job: dict) -> tuple[bool, str, str, Optional[str]]:
24582545
)
24592546
_job_workdir = None
24602547
_prior_terminal_cwd = os.environ.get("TERMINAL_CWD", "_UNSET_")
2548+
_workdir_file_redirect = None
24612549
if _job_workdir:
24622550
os.environ["TERMINAL_CWD"] = _job_workdir
24632551
logger.info("Job '%s': using workdir %s", job_id, _job_workdir)
2552+
# TERMINAL_CWD above only redirects the terminal/git tools. Also point
2553+
# the file-edit tools at the workdir so relative writes land there
2554+
# instead of dirtying the runtime checkout (split-brain — see
2555+
# _seed_workdir_file_tools). Scoped + restored in finally, exactly like
2556+
# TERMINAL_CWD; tick() serializes workdir jobs so the shared in-process
2557+
# env state is safe to mutate for the duration of this job.
2558+
_workdir_file_redirect = _seed_workdir_file_tools(_job_workdir)
24642559

24652560
try:
24662561
# Re-read .env and config.yaml fresh every run so provider/key
@@ -3037,6 +3132,9 @@ def _run_job_impl(job: dict) -> tuple[bool, str, str, Optional[str]]:
30373132
os.environ.pop("TERMINAL_CWD", None)
30383133
else:
30393134
os.environ["TERMINAL_CWD"] = _prior_terminal_cwd
3135+
# Undo the file-edit-tool workdir redirect (cwd override + live env
3136+
# cwd), restoring the shared "default" env to its pre-job state.
3137+
_restore_workdir_file_tools(_workdir_file_redirect)
30403138
# Clean up ContextVar session/delivery state for this job.
30413139
clear_session_vars(_ctx_tokens)
30423140
for _var_name in _cron_delivery_vars:

0 commit comments

Comments
 (0)