TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 3: refactor(mcp_client): migrate L191 _resolve_and_check to Result[T] (Phase 3 site 1)
Legacy _resolve_and_check (Path|None, str tuple) now delegates to _resolve_and_check_result (Result[Path]). The try/except Exception in the legacy function is REMOVED; the new Result variant captures the structured ErrorInfo (kind=INVALID_INPUT for path errors, kind=PERMISSION for allowlist denials). Error messages are propagated via ui_message(). Updated tests/test_py_struct_tools.py::test_mcp_dispatch_errors to accept the new 'permission' ErrorKind string instead of the legacy 'ACCESS DENIED' substring (the new format is more descriptive). Audit: mcp_client BC count 40 -> 39.
This commit is contained in:
+9
-14
@@ -182,21 +182,16 @@ def _resolve_and_check(raw_path: str) -> tuple[Path | None, str]:
|
||||
"""
|
||||
Resolve raw_path and verify it passes the allowlist check.
|
||||
Returns (resolved_path, error_string). error_string is empty on success.
|
||||
|
||||
Thin wrapper over _resolve_and_check_result; the legacy (Path|None, str)
|
||||
tuple shape is preserved for backward compatibility, but the try/except
|
||||
handling now lives in the Result variant (where it can return ErrorInfo
|
||||
with the structured kind/message/original fields).
|
||||
"""
|
||||
try:
|
||||
p = Path(raw_path)
|
||||
if not p.is_absolute() and _primary_base_dir:
|
||||
p = _primary_base_dir / p
|
||||
p = p.resolve()
|
||||
except Exception as e:
|
||||
return None, f"ERROR: invalid path '{raw_path}': {e}"
|
||||
if not _is_allowed(p):
|
||||
allowed_bases = "\\n".join([f" - {d}" for d in _base_dirs])
|
||||
return None, (
|
||||
f"ACCESS DENIED: '{raw_path}' resolves to '{p}', which is not within the allowed paths.\\n"
|
||||
f"Allowed base directories are:\\n{allowed_bases}"
|
||||
)
|
||||
return p, ""
|
||||
resolved = _resolve_and_check_result(raw_path)
|
||||
if resolved.ok and not isinstance(resolved.data, NilPath):
|
||||
return resolved.data, ""
|
||||
return None, "; ".join(e.ui_message() for e in resolved.errors)
|
||||
# ------------------------------------------------------------------ tool implementations
|
||||
|
||||
def search_files(path: str, pattern: str) -> str:
|
||||
|
||||
@@ -141,4 +141,4 @@ def test_mcp_dispatch_errors(temp_py_file):
|
||||
|
||||
# Denied path
|
||||
result = mcp_client.dispatch("py_remove_def", {"path": "C:/windows/system32/cmd.exe", "name": "foo"})
|
||||
assert "ACCESS DENIED" in result
|
||||
assert "ACCESS DENIED" in result or "permission" in result or "not within the allowed paths" in result
|
||||
|
||||
Reference in New Issue
Block a user