fix(cli): forward --worktree to run_oneshot so -z -w works (#67458) - #67484
fix(cli): forward --worktree to run_oneshot so -z -w works (#67458)#67484webtecnica wants to merge 1 commit into
Conversation
…rch#67458) The --worktree/-w flag was silently ignored in one-shot mode (-z): run_oneshot() didn't accept a worktree parameter, and neither call site in main.py forwarded the parsed flag. Commits landed on the current branch instead of an isolated worktree. - Add to run_oneshot() in hermes_cli/oneshot.py - Wire worktree setup (git_repo_root, prune_stale_worktrees, _setup_worktree) and cleanup in oneshot.py, mirroring the interactive-mode path in cli.py - Set HERMES_CWD and TERMINAL_CWD so the agent runs inside the worktree directory - Clean up the worktree in the finally block (preserving it when it has unpushed commits, same policy as interactive mode) - Pass worktree=getattr(args, 'worktree', False) from both run_oneshot() call sites in main.py
teknium1
left a comment
There was a problem hiding this comment.
Thanks for covering both one-shot dispatch paths; the current-main omission is real (hermes_cli/main.py:12970-12976 and 15127-15133).
Problems
hermes_cli/oneshot.py:251invokes_setup_worktree()before stdout redirection, but that helper prints creation details (cli.py:1630-1632). The added cleanup at line 303 also prints to stdout (cli.py:1776-1777,1811). This breaks the documented-zcontract that stdout is the final response only (hermes_cli/main.py:15121-15122).- Lines 257-258 replace
HERMES_CWDandTERMINAL_CWD, but the finally block does not restore them. After cleanup removes the worktree, an in-process caller can retain a danglingTERMINAL_CWD, whichagent/runtime_cwd.py:67-73consumes.
Suggested changes
- Redirect setup and cleanup notices to stderr and restore both environment variables in the final cleanup path.
- Add forwarding and lifecycle regressions for both dispatches, stdout purity, setup failure, and environment restoration.
Automated hermes-sweeper review.
| repo = _git_repo_root() | ||
| if repo: | ||
| _prune_stale_worktrees(repo) | ||
| _wt_info = _setup_worktree() |
There was a problem hiding this comment.
_setup_worktree() prints creation messages to stdout (cli.py:1630-1632), but -z promises stdout contains only the final response. Route this helper's output to stderr before calling it.
| ) | ||
| return 1 | ||
| os.environ["HERMES_CWD"] = _wt_info["path"] | ||
| os.environ["TERMINAL_CWD"] = _wt_info["path"] |
There was a problem hiding this comment.
Save the prior CWD environment values and restore them in the final cleanup path. As written, a direct caller retains TERMINAL_CWD after _cleanup_worktree() may delete this path.
| if _wt_info is not None: | ||
| try: | ||
| from cli import _cleanup_worktree | ||
| _cleanup_worktree(_wt_info) |
There was a problem hiding this comment.
_cleanup_worktree() prints both normal cleanup and unpushed-commit notices to stdout (cli.py:1776-1777, 1811). Redirect this call to real_stderr so it cannot contaminate one-shot output.
SummaryThree PRs address #67458 by forwarding Related pull requests
Duplicates#67471 and #67476 substantially implement the same honor- Suggested consolidationKeep #67476 open with a salvage path preserving its two dispatch forwards, fail-closed setup, clean stdout, Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I67458(["issue #67458 (open)"])
subgraph Dup67471 ["PRs duplicating each other"]
P67471["PR #67471 (open)"]
P67476["PR #67476 (open)"]
P67484["PR #67484 (open)"]
end
P67484 -.->|partial| I67458
class I67458 open
class P67471 open
class P67476 open
class P67484 open
class P67476 best
class P67484 target
click I67458 "https://github.com/NousResearch/hermes-agent/issues/67458"
click P67471 "https://github.com/NousResearch/hermes-agent/pull/67471"
click P67476 "https://github.com/NousResearch/hermes-agent/pull/67476"
click P67484 "https://github.com/NousResearch/hermes-agent/pull/67484"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 3 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 72 kB of PR diffs, 11 kB of issue/PR text, 10 kB of discussion (14 comments), 5 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Closes #67458.
Problem
The
--worktree/-wflag was silently ignored in one-shot mode (-z). Runninghermes -z "make a commit" -wwould commit directly to the current branch instead of using an isolated git worktree.Root cause
run_oneshot()inhermes_cli/oneshot.pydidn't accept aworktreeparameter, and neither call site inhermes_cli/main.pyforwarded the parsedworktreeflag.Fix
worktree: bool = Falseparameter torun_oneshot()cli.py:_git_repo_root,_prune_stale_worktrees,_setup_worktree) and cleanup (_cleanup_worktree) — mirroring the interactive-mode pathHERMES_CWDandTERMINAL_CWDenv vars so the agent runs inside the worktreefinallyblock (preserving it when it has unpushed commits — same policy as interactive mode)worktree=getattr(args, "worktree", False)from bothrun_oneshot()call sites inmain.pyTesting
cli.pyare already covered bytests/cli/test_worktree.py