fix(agfs): fix agfs binding-client import error

[chuanbao666]

Summary

修复agfs binding-client无法import问题。
binding-client目前未合入到agfs主干，pyagfs 1.4.0不能找到该class。
修复方案：复制pyagfs到openviking，改用openviking.pyagfs

Type of Change

• New feature (feat)
• Bug fix (fix)
• Documentation (docs)
• Refactoring (refactor)
• Other

Testing

描述如何测试这些更改：

• Unit tests pass
• Manual testing completed

Related Issues

• Related to pyagfs compatibility issue: AGFSBindingClient not found #456

Checklist

• Code follows project style guidelines
• Tests added for new functionality
• Documentation updated (if needed)
• All tests pass

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 1f47239)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 456 - Partially compliant Compliant requirements: Fixed ImportError by copying pyagfs into openviking.pyagfs and updating all imports Non-compliant requirements: No documentation added about compatible pyagfs versions Requires further human verification: Verify that both http-client and binding-client modes work correctly Verify that the native library (libagfsbinding) is properly located/built when using binding-client mode

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Default Mode Change The default AGFS client mode was changed from 'http-client' to 'binding-client'. This may cause issues if the native library (libagfsbinding) is not available. Verify if this change is intentional and properly documented. mode: str = Field( default="binding-client", description="AGFS client mode: 'http-client' | 'binding-client'", Private Function Usage The code imports and uses the private function '_find_library' from openviking.pyagfs.binding_client. While the code is copied into the project, using private functions reduces maintainability. Consider if there's a public API alternative or if this is acceptable. from openviking.pyagfs.binding_client import _find_library

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 1f47239
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Free native memory in cat/read method Free the native memory allocated for result_ptr using AGFS_FreeString after copying the data. This prevents a memory leak each time cat/read is called. openviking/pyagfs/binding_client.py [277-281] if result_ptr: data = ctypes.string_at(result_ptr, size_ptr.value) + self._lib.lib.AGFS_FreeString(result_ptr) return data return b"" Suggestion importance[1-10]: 8 __ Why: Fixes a critical memory leak by freeing native memory allocated for result_ptr using AGFS_FreeString after copying the data, which is important for long-running processes using the binding client.	Medium
General	Update outdated import error message Update the ImportError message to reflect that pyagfs is now included as openviking.pyagfs and no longer needs separate installation. openviking/init.py [30-33] except ImportError: raise ImportError( - "pyagfs not found. Please install: pip install -e third_party/agfs/agfs-sdk/python" + "Failed to import AGFS client. This should be included with openviking." ) Suggestion importance[1-10]: 4 __ Why: Updates the outdated ImportError message to reflect that pyagfs is now included as openviking.pyagfs, improving user experience but not affecting core functionality.	Low

---

Previous suggestions

Suggestions up to commit 077380c

Category	Suggestion	Impact
Possible issue	Fix memory leak in cat method Fix memory leak in the cat method by freeing the library-allocated result_ptr using AGFS_FreeString. This prevents memory leaks when reading files with the binding client. openviking/pyagfs/binding_client.py [277-281] if result_ptr: data = ctypes.string_at(result_ptr, size_ptr.value) + self._lib.lib.AGFS_FreeString(result_ptr) return data return b"" Suggestion importance[1-10]: 8 __ Why: This correctly identifies and fixes a memory leak in the cat method by freeing the library-allocated result_ptr with AGFS_FreeString, which is important for preventing memory leaks when using the binding client.	Medium
	Fix memory leaks in _parse_response Fix memory leaks in all methods using _parse_response by modifying it to accept and free library-allocated strings. Since ctypes converts c_char_p to bytes immediately, we need to adjust how we call library functions to retain the pointer for freeing. openviking/pyagfs/binding_client.py [221-233] -def _parse_response(self, result: bytes) -> Dict[str, Any]: - """Parse JSON response from the library.""" - if isinstance(result, bytes): - result = result.decode("utf-8") - data = json.loads(result) +def _parse_response(self, result_ptr: ctypes.c_char_p) -> Dict[str, Any]: + """Parse JSON response from the library and free the allocated string.""" + try: + if not result_ptr: + return {} + result_bytes = ctypes.string_at(result_ptr) + result = result_bytes.decode("utf-8") + data = json.loads(result) - if "error_id" in data and data["error_id"] != 0: - error_msg = self._lib.lib.AGFS_GetLastError(data["error_id"]) - if isinstance(error_msg, bytes): - error_msg = error_msg.decode("utf-8") - raise AGFSClientError(error_msg if error_msg else "Unknown error") + if "error_id" in data and data["error_id"] != 0: + error_msg = self._lib.lib.AGFS_GetLastError(data["error_id"]) + if isinstance(error_msg, bytes): + error_msg = error_msg.decode("utf-8") + raise AGFSClientError(error_msg if error_msg else "Unknown error") - return data + return data + finally: + if result_ptr: + self._lib.lib.AGFS_FreeString(result_ptr) Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a memory leak issue with library-allocated strings, but the proposed change would break all existing callers of _parse_response since they currently pass bytes objects. Additionally, ctypes would need to be configured differently to retain raw pointers instead of auto-converting to bytes.	Low
	Update methods to use new _parse_response Update all methods calling library functions that return strings to work with the new _parse_response signature. Since we need the raw pointer to free it, we must adjust our ctypes calls to retain the pointer. openviking/pyagfs/binding_client.py [240-249] def get_capabilities(self) -> Dict[str, Any]: """Get client capabilities.""" - result = self._lib.lib.AGFS_GetCapabilities(self._client_id) - return self._parse_response(result) + self._lib.lib.AGFS_GetCapabilities.restype = ctypes.c_char_p + result_ptr = self._lib.lib.AGFS_GetCapabilities(self._client_id) + return self._parse_response(result_ptr) def ls(self, path: str = "/") -> List[Dict[str, Any]]: """List directory contents.""" - result = self._lib.lib.AGFS_Ls(self._client_id, path.encode("utf-8")) - data = self._parse_response(result) + self._lib.lib.AGFS_Ls.restype = ctypes.c_char_p + result_ptr = self._lib.lib.AGFS_Ls(self._client_id, path.encode("utf-8")) + data = self._parse_response(result_ptr) return data.get("files", []) Suggestion importance[1-10]: 5 __ Why: The suggestion is directionally correct for addressing memory leaks, but the implementation is incomplete (only updates two methods) and the restype is already set in _setup_functions. Also, it doesn't fully resolve how to retain raw pointers from ctypes calls.	Low

[github-actions]

Persistent review updated to latest commit 1f47239