Skip to content

fix: isolate file tool paths and cron script workdirs - #64894

Open
TMShiroiko wants to merge 4 commits into
NousResearch:mainfrom
TMShiroiko:codex-sprint/validated-fixes
Open

fix: isolate file tool paths and cron script workdirs#64894
TMShiroiko wants to merge 4 commits into
NousResearch:mainfrom
TMShiroiko:codex-sprint/validated-fixes

Conversation

@TMShiroiko

Copy link
Copy Markdown

Summary

  • Resolve local read_file and search_files backend paths against the task's own validated root before dispatch, preventing a cross-task backend-CWD escape.
  • Run cron scripts in the configured workdir, falling back to the script directory when unset or invalid; child CWD selection does not use process-global os.chdir.
  • Preserve the legacy one-argument script-runner seam when no execution CWD is selected.
  • Keep the read-loop regression expectation portable across POSIX/macOS and native Windows path contracts.

Validation

Focused suites passed:

  • tests/tools/test_file_tools.py -k 'TaskRootReadDispatch or WindowsMsysPathResolution': 6 passed
  • tests/tools/test_read_loop_detection.py: 25 passed
  • tests/cron/test_cron_no_agent.py tests/cron/test_cron_workdir.py tests/cron/test_cron_script.py: 79 passed
  • tests/cron/test_script_claim_heartbeat.py: 3 passed
  • tests/cron/test_scheduler.py::TestRunJobWakeGate::test_script_runs_only_once_on_wake: 1 passed
  • heartbeat-related suites: 8 passed
  • git diff --check: passed

Full suite was run before and after the integration fixes. It is not clean on this macOS environment due to pre-existing platform/config/timing failures and 11 collection/import failures.

  • Base audit SHA 46e87b14fd6c943ef0d6671fb0d74c5dde5d4c6b: 36 failures across 17 files + 11 collection/import failures.
  • Final integration run: 39 failures across 20 files + the same 11 collection/import failures. The original SEC read-loop and CRON heartbeat/scheduler regressions no longer appear.
  • The remaining apparent delta is environmental/flaky: the Honcho client and web-memory nodes reproduce identically against base and integration with fresh HERMES_HOME; the approved-command interrupt node passed 3/3 direct runs in both worktrees; the atomic snapshot node flakes on base (1/3 failure).

Scope

No configuration, lockfile, credentials, or global-CWD behavior changed.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management tool/file File tools (read, write, patch, search) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fixes. The reported behavior is present on current main: tools/file_tools.py:1366 and tools/file_tools.py:1995-1998 dispatch the original relative path after task-root resolution, while cron/scheduler.py:2106 always runs scripts in their script directory and the no-agent path mutates process-global CWD at cron/scheduler.py:2601-2617.

The PR changes dispatch to the validated path, passes an explicit child CWD through both normal and heartbeat script paths, and covers both no-agent and agent pre-run execution. Current production run_job() passes the pre-run result into _build_job_prompt() (cron/scheduler.py:2755-2774), so the normal agent path does not re-run the script through the legacy direct helper branch.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Six PRs are associated with this three-issue guardrail complex. #34614, #34778, and #43930 address repeated successful mutations; #41492 adds file-tool recovery hints; #57303 adds runtime-enforced soft blocks for repeated failures and read-only no-progress loops; and #64894 independently fixes task-root path dispatch and cron subprocess working directories.

Related pull requests

Duplicates

#34778 is the closed duplicate of #34614 for successful mutating-call repetition. #43930 overlaps that mechanism but bundles a distinct overwrite guard; #41492 and #57303 overlap on recovery guidance but are not strict duplicates because #57303 adds shared runtime enforcement.

Suggested consolidation

Author action: rebase #57303 onto main, or split out its default soft-block logic, rotating guidance, configuration/docs changes, and sequential/concurrent runtime tests for fresh focused review; this preserves its recorded best-fix status and visible keep_open review. Keep #34614 open with its reviewed salvage work, keep #41492 open for its complementary action_hint path and missing dedup assertion, retain #43930 and #64894 as separate salvage tracks, and leave #34778 closed as duplicate of #34614.

Cross-PR triage: Reviewed 6 pull requests and 3 issues in this complex. Each diff was read against this issue; Assessment working set: 83 kB of PR diffs, 37 kB of issue/PR text, 7 kB of discussion (13 comments), 12 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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

Labels

comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/file File tools (read, write, patch, search) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants