-
Notifications
You must be signed in to change notification settings - Fork 192
fix: basic memory home env var not respected when project path is changed. #239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -309,6 +309,47 @@ async def synchronize_projects(self) -> None: # pragma: no cover | |
| # MCP components might not be available in all contexts | ||
| logger.debug("MCP session not available, skipping session refresh") | ||
|
|
||
| async def move_project(self, name: str, new_path: str) -> None: | ||
| """Move a project to a new location. | ||
|
|
||
| Args: | ||
| name: The name of the project to move | ||
| new_path: The new absolute path for the project | ||
|
|
||
| Raises: | ||
| ValueError: If the project doesn't exist or repository isn't initialized | ||
| """ | ||
| if not self.repository: | ||
| raise ValueError("Repository is required for move_project") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks like Claude doing goldplating BS. We should always have a repository. But whatever.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah seems overkill. All the other service method have it, so that's probably why Claude did it here. |
||
|
|
||
| # Resolve to absolute path | ||
| resolved_path = os.path.abspath(os.path.expanduser(new_path)) | ||
|
|
||
| # Validate project exists in config | ||
| if name not in self.config_manager.projects: | ||
| raise ValueError(f"Project '{name}' not found in configuration") | ||
|
|
||
| # Create the new directory if it doesn't exist | ||
| Path(resolved_path).mkdir(parents=True, exist_ok=True) | ||
|
|
||
| # Update in configuration | ||
| config = self.config_manager.load_config() | ||
| old_path = config.projects[name] | ||
| config.projects[name] = resolved_path | ||
| self.config_manager.save_config(config) | ||
|
|
||
| # Update in database | ||
| project = await self.repository.get_by_name(name) | ||
| if project: | ||
| await self.repository.update_path(project.id, resolved_path) | ||
| logger.info(f"Moved project '{name}' from {old_path} to {resolved_path}") | ||
| else: | ||
| logger.error(f"Project '{name}' exists in config but not in database") | ||
| # Restore the old path in config since DB update failed | ||
| config.projects[name] = old_path | ||
| self.config_manager.save_config(config) | ||
| raise ValueError(f"Project '{name}' not found in database") | ||
|
|
||
| async def update_project( # pragma: no cover | ||
| self, name: str, updated_path: Optional[str] = None, is_active: Optional[bool] = None | ||
| ) -> None: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -146,19 +146,19 @@ def setup_logging( | |
| # logger.remove() | ||
|
|
||
| # Add file handler if we are not running tests and a log file is specified | ||
| # if log_file and env != "test": | ||
| # # Setup file logger | ||
| # log_path = home_dir / log_file | ||
| # logger.add( | ||
| # str(log_path), | ||
| # level=log_level, | ||
| # rotation="10 MB", | ||
| # retention="10 days", | ||
| # backtrace=True, | ||
| # diagnose=True, | ||
| # enqueue=True, | ||
| # colorize=False, | ||
| # ) | ||
| if log_file and env != "test": | ||
| # Setup file logger | ||
| log_path = home_dir / log_file | ||
| logger.add( | ||
| str(log_path), | ||
| level=log_level, | ||
| rotation="10 MB", | ||
| retention="10 days", | ||
| backtrace=True, | ||
| diagnose=True, | ||
| enqueue=True, | ||
| colorize=False, | ||
| ) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. re-enable logging to log file |
||
|
|
||
| # Add console logger if requested or in test mode | ||
| # if env == "test" or console: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this fixes the issue with config file being overwritten by default values. Just propagate the error.