fix(cli): honor explicit local working directory - #65286
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the classic CLI bridge. The runtime premise is confirmed: current main overwrites local terminal.cwd with os.getcwd() in cli.py:603-605.
Problems
tests/cli/test_cwd_env_respect.py:20duplicates the proposed branch in_resolve_cwdinstead of invokingcli.load_cli_config. It can remain green if the production loader changes independently. The repository requires behavior tests rather than source-shape/copy tests (AGENTS.md:1370-1404).- This does not yet close #51636 as written. That issue reproduces with
terminal.working_dir, while the runtime readsterminal.cwd(cli.py:603-605); the config env map has onlycwd(hermes_cli/config.py:6905-6933) andconfig setdoes not normalize that alias (hermes_cli/config.py:8291-8300).
Suggested changes
- Test the real loader with an isolated config and patched launch directory.
- Either support/migrate
terminal.working_dirwith coverage, or removeCloses #51636and keep this scoped to explicitterminal.cwd.
Automated hermes-sweeper review.
|
|
||
| if effective_backend == "local": | ||
| terminal_config["cwd"] = "/fake/getcwd" | ||
| configured_cwd = terminal_config.get("cwd") |
There was a problem hiding this comment.
This helper duplicates the production branch rather than exercising cli.load_cli_config, so the test can pass even if the real loader diverges. Please replace it with an isolated config fixture that imports the real loader, patches its Hermes home and launch cwd, and asserts the resulting terminal.cwd/TERMINAL_CWD behavior.
Summary33 PRs address or reference this issue complex across shell-snapshot CWD capture, CLI/Desktop CWD policy, prompt CWD reporting, the terminal.working_dir alias, and an unrelated personality-overlay path. For #51636 specifically, #51645 handles the reported alias while #65286 handles explicit terminal.cwd and setup defaults, but each visible contributor review identifies a remaining gap. Related pull requests
Duplicates#7802, #9798, #11708, #11878, #14796, #14828, #14830, #14831, and #16762 substantially overlap with the bootstrap fix landed in #17205; #24888, #24957, #24985, and #31206 overlap with the prompt-reader fix consolidated in #35028. #42984, #43075, #59340, and #63092 share the explicit-local-CWD guard, while #51645 and #65286 are complementary rather than duplicates. Suggested consolidationAuthor action: rebase #51645 onto main and extend its terminal.working_dir normalization to the direct gateway and TUI YAML readers with regressions, exactly addressing its contributor keep_open review; it remains the recorded best fix for #51636. For #65286, address its keep_open review by replacing mirrored tests with real load_cli_config coverage and either incorporate the alias path without conflicting with #51645 or remove Closes #51636 and split the explicit-terminal.cwd/setup-default change into its own scoped issue; close #43075 as duplicate of #42984 if that separate CWD-contract work consolidates there. Cross-PR triage: Reviewed 33 pull requests and 11 issues in this complex. Each diff was read against this issue; Assessment working set: 216 kB of PR diffs, 90 kB of issue/PR text, 49 kB of discussion (83 comments), 57 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Closes #51636
Summary:
Verification: