fix(agent): guard coding-context cwd fallback against getcwd errors - #45423
fix(agent): guard coding-context cwd fallback against getcwd errors#45423Frowtek wants to merge 1 commit into
Conversation
_resolve_cwd() fell back to Path(os.getcwd()) when resolve_agent_cwd() raised. But resolve_agent_cwd() raises precisely because os.getcwd() failed (a deleted CWD with no TERMINAL_CWD/session override), so the fallback re-invoked the same failing syscall and propagated out of resolve_runtime_mode() and build_coding_workspace_block(). Path.resolve() also calls os.getcwd() for absolute paths on some platforms, making _git_root/_marker_root/_home and the worktree probe additional crash sites. Add _safe_resolve() — a resolve() with a getcwd-free normpath fallback on OSError — and use it at every resolve() in the resolution path. Degrade _resolve_cwd() to home, then the temp dir, when getcwd fails, so posture detection settles on general/no-workspace instead of crashing prompt build. resolve_agent_cwd()'s documented OSError contract is unchanged; this only fixes its caller-side guard. Adds TestDeletedCwdTolerance covering posture resolution and workspace block building under a raising os.getcwd(), plus the explicit-cwd and home/temp fallback paths.
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Adds _safe_resolve() wrapper around Path.resolve() to tolerate a vanished/inaccessible CWD (Windows resolve() calls os.getcwd() even for absolute paths). The fallback degrades gracefully instead of crashing posture detection on every prompt build.
Strengths:
- Well-documented with clear explanation of the Windows-specific behavior
- Comprehensive test coverage: deleted CWD, explicit cwd bypass, home fallback, tempdir fallback
_resolve_cwd()now has a clear contract (never raises) and the tests confirm it- Changes are isolated to the affected functions
No blocking issues found.
Reviewed by Hermes Agent
|
Thanks for the focused regression fix. The premise is verified against current main: The PR's guarded fallback and This is an automated hermes-sweeper review. |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs form this complex: #43316 introduced the coding-context posture and its CWD-resolution path, while #45423 fixes the resulting deleted/inaccessible-CWD crash by replacing the repeated failing getcwd fallback with guarded home/temp-directory degradation and safe path resolution.
Related pull requests
- #43316 [merged]
related— (+1319/-4) — merged foundation: #43316 remains relevant as the merged implementation that introduced agent/coding_context.py, including the fallback path that could repeat a failing os.getcwd() call. - #45423
related— (+117/-4) — merge: #45423 directly fixes the root cause by adding getcwd-tolerant path resolution, degrading an unavailable implicit CWD to home or the temp directory, and covering runtime-mode and workspace-block paths with six regression tests. The contributor keep_open review explicitly verified the premise against current main and found no correctness issue; the separate APPROVED review also found no blocking issue but did not document a current-main verification.
Suggested consolidation
Merge #45423 — it is a focused regression fix for the CWD failure path introduced by the merged #43316, with both affected consumers and additional resolve() crash sites covered; this aligns with the contributor keep_open review. #43316 is the merged reference implementation, not a duplicate, so no PR should be closed as duplicative.
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 72 kB of PR diffs, 12 kB of issue/PR text, 18 kB of discussion (7 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
What does this PR do?
_resolve_cwd()inagent/coding_context.pyfell back toPath(os.getcwd())whenresolve_agent_cwd()raised — butresolve_agent_cwd()raises becauseos.getcwd()failed (a deletedCWD with no
TERMINAL_CWD/session override). The fallback re-invoked thesame failing syscall and propagated out of
resolve_runtime_mode()andbuild_coding_workspace_block(), crashing prompt build.Path.resolve()can also callos.getcwd()for absolute paths, so_git_root,_marker_root,_home, and the worktree probe wereadditional crash sites.
The fix adds
_safe_resolve()(aresolve()with a getcwd-freenormpathfallback onOSError) and uses it at everyresolve()in theresolution path.
_resolve_cwd()now degrades to home, then the tempdir, so posture detection settles on general/no-workspace instead of
crashing.
resolve_agent_cwd()'s documentedOSErrorcontract isunchanged — only its caller-side guard is fixed.
Related Issue
N/A — found via code audit.
Type of Change
Changes Made
agent/coding_context.py: add_safe_resolve(); guard the_resolve_cwd()fallback so it never re-calls a failingos.getcwd()(degrades to home → temp dir); route
_git_root,_marker_root,_home, and the worktree probe through_safe_resolve().tests/agent/test_coding_context.py: addTestDeletedCwdTolerance(6 tests).
How to Test
Result:
The new tests fail against the unpatched code (
_resolve_cwdre-raisesFileNotFoundError) and pass after the fix. Consumer/integration suitesalso pass:
pytest tests/agent/test_system_prompt.py tests/agent/test_prompt_builder.py \ tests/test_tui_gateway_server.py -q # 473 passed, 1 skippedChecklist
Code
Documentation & Housekeeping
cli-config.yaml.example— N/ACONTRIBUTING.md/AGENTS.md— N/A