From bc1c6c5e6611464d92ae23d95d224796c4b76b1b Mon Sep 17 00:00:00 2001 From: PeterShin23 Date: Tue, 16 Dec 2025 22:39:34 -0500 Subject: [PATCH 1/9] Add project from slack --- src/chat_adapters/i_chat_adapter.py | 11 +- src/chat_adapters/slack_adapter.py | 11 +- src/core/commands/project_creation.py | 186 +++++++++++ src/core/config.py | 10 +- src/core/errors.py | 5 + src/core/project_creation.py | 310 ++++++++++++++++++ src/core/router.py | 29 +- tests/commands/conftest.py | 4 +- tests/test_project_creation.py | 242 ++++++++++++++ tests/test_router_integration.py | 8 +- tests/test_router_reactions.py | 451 ++++++++++++++++++++++++++ 11 files changed, 1254 insertions(+), 13 deletions(-) create mode 100644 src/core/commands/project_creation.py create mode 100644 src/core/project_creation.py create mode 100644 tests/test_project_creation.py create mode 100644 tests/test_router_reactions.py diff --git a/src/chat_adapters/i_chat_adapter.py b/src/chat_adapters/i_chat_adapter.py index e50a558..96394a7 100644 --- a/src/chat_adapters/i_chat_adapter.py +++ b/src/chat_adapters/i_chat_adapter.py @@ -3,14 +3,21 @@ from __future__ import annotations import abc +from typing import Optional class IChatAdapter(abc.ABC): """Abstraction for chat platform integrations (Slack, Discord, etc.).""" @abc.abstractmethod - async def send_message(self, channel: str, thread_ts: str, text: str) -> None: - """Send a message to a channel/thread.""" + async def send_message( + self, channel: str, thread_ts: str, text: str + ) -> Optional[str]: + """Send a message to a channel/thread. + + Returns: + The message timestamp/ID if available, None otherwise. + """ @abc.abstractmethod async def start(self) -> None: diff --git a/src/chat_adapters/slack_adapter.py b/src/chat_adapters/slack_adapter.py index a648793..2afaef4 100644 --- a/src/chat_adapters/slack_adapter.py +++ b/src/chat_adapters/slack_adapter.py @@ -4,7 +4,7 @@ import asyncio import logging -from typing import Any, Dict +from typing import Any, Dict, Optional from slack_sdk.errors import SlackApiError from slack_sdk.socket_mode.aiohttp import SocketModeClient @@ -35,9 +35,14 @@ def __init__( self._channel_name_cache: Dict[str, str] = {} self._client.socket_mode_request_listeners.append(self._handle_socket_request) - async def send_message(self, channel: str, thread_ts: str, text: str) -> None: + async def send_message( + self, channel: str, thread_ts: str, text: str + ) -> Optional[str]: try: - await self._web_client.chat_postMessage(channel=channel, text=text, thread_ts=thread_ts) + response = await self._web_client.chat_postMessage( + channel=channel, text=text, thread_ts=thread_ts + ) + return response.get("ts") except SlackApiError as exc: raise SlackError(f"Failed to send Slack message: {exc}") from exc diff --git a/src/core/commands/project_creation.py b/src/core/commands/project_creation.py new file mode 100644 index 0000000..5624d86 --- /dev/null +++ b/src/core/commands/project_creation.py @@ -0,0 +1,186 @@ +"""Handler for automatic project creation from Slack.""" + +from __future__ import annotations + +import logging +import time +from dataclasses import dataclass +from typing import Awaitable, Callable, Dict, Optional + +from ..config import Config, load_config +from ..project_creation import ProjectCreationRequest, ProjectCreationService +from ...github import GitHubManager + +LOGGER = logging.getLogger(__name__) + +# Type alias for the message sender callback +SendMessage = Callable[[str, str, str], Awaitable[Optional[str]]] + + +@dataclass +class PendingProjectCreation: + """Tracks a pending project creation prompt.""" + + channel_id: str + channel_name: str + thread_ts: str + created_at: float + + +class ProjectCreationHandler: + """Handles automatic project creation when messaging unknown channels.""" + + def __init__( + self, + config: Config, + github_manager: GitHubManager, + config_root, + ) -> None: + self._config = config + self._config_root = config_root + self._pending_projects: Dict[str, PendingProjectCreation] = {} + self._project_creator = ProjectCreationService( + config=config, + github_manager=github_manager, + ) + + def update_config(self, config: Config) -> None: + """Update the config reference.""" + self._config = config + self._project_creator.update_config(config) + + async def handle_missing_project( + self, + channel_id: str, + channel_name: str, + thread_ts: str, + send_message: SendMessage, + ) -> None: + """ + Handle a message to a channel without a configured project. + + Sends a prompt asking the user if they want to create the project. + """ + # Check if we already prompted for this channel + if channel_id in self._pending_projects: + LOGGER.debug("Project creation already pending for %s", channel_name) + return + + # Send prompt message + await send_message( + channel_id, + thread_ts, + f"I couldn't find a project named **{channel_name}**. Is this a new idea?!?!\n\n" + "Reply with **Y** to create it, or **N** to cancel.", + ) + + # Track pending creation + self._pending_projects[channel_id] = PendingProjectCreation( + channel_id=channel_id, + channel_name=channel_name, + thread_ts=thread_ts, + created_at=time.time(), + ) + + async def handle_response( + self, + channel_id: str, + text: str, + send_message: SendMessage, + ) -> tuple[bool, Optional[Config]]: + """ + Handle a potential Y/N response to a pending project creation prompt. + + Args: + channel_id: The channel ID + text: The message text + send_message: Callback to send messages + + Returns: + Tuple of (was_handled, new_config_if_created) + - was_handled: True if this was a response to a pending prompt + - new_config_if_created: New Config if project was created, None otherwise + """ + pending = self._pending_projects.get(channel_id) + if not pending: + return False, None + + response = text.strip().lower() + + if response in ("y", "yes"): + del self._pending_projects[channel_id] + new_config = await self._handle_approval(pending, send_message) + return True, new_config + elif response in ("n", "no"): + del self._pending_projects[channel_id] + await self._handle_rejection(pending, send_message) + return True, None + else: + # Not a y/n response, remind them + await send_message( + pending.channel_id, + pending.thread_ts, + f"Please reply with **Y** to create project **{pending.channel_name}**, or **N** to cancel.", + ) + return True, None + + async def _handle_approval( + self, + pending: PendingProjectCreation, + send_message: SendMessage, + ) -> Optional[Config]: + """ + Handle user approving project creation. + + Returns: + New Config if successful, None on failure + """ + await send_message( + pending.channel_id, + pending.thread_ts, + f"Creating project **{pending.channel_name}**...", + ) + + try: + request = ProjectCreationRequest( + project_id=pending.channel_name, + channel_name=pending.channel_name, + default_agent_id="claude", + ) + + await self._project_creator.create_project(request) + + # Reload config + new_config = load_config(self._config_root) + + await send_message( + pending.channel_id, + pending.thread_ts, + f"Okay great, I've set up **{pending.channel_name}**! " + "What's this new idea? What do you want me to do?", + ) + + return new_config + + except Exception as e: + LOGGER.exception("Failed to create project %s", pending.channel_name) + await send_message( + pending.channel_id, + pending.thread_ts, + f"Sorry, I couldn't create the project: {e}", + ) + return None + + async def _handle_rejection( + self, + pending: PendingProjectCreation, + send_message: SendMessage, + ) -> None: + """Handle user rejecting project creation.""" + await send_message( + pending.channel_id, + pending.thread_ts, + "Okay, if the project name is different, then rename the channel " + "and reload to try again. Otherwise, this channel won't be connected " + "to any of your repositories.", + ) diff --git a/src/core/config.py b/src/core/config.py index e569cd3..8a9ba2a 100644 --- a/src/core/config.py +++ b/src/core/config.py @@ -29,6 +29,8 @@ class Config: slack_bot_token: str slack_app_token: str slack_allowed_user_ids: list[str] + base_dir: Path + config_dir: Path github_token: str | None = None def get_project_by_channel(self, channel: str) -> Project: @@ -74,7 +76,7 @@ def load_config(config_dir: Path | str | None = None) -> Config: def _load_config_from_root(root: Path) -> Config: _load_env_file(root / ENV_FILE_NAME) - projects = _load_projects(root / PROJECTS_FILE) + projects, base_dir = _load_projects(root / PROJECTS_FILE) agents = _select_agents(_load_agents(root / AGENTS_FILE)) slack_bot_token = _require_env("SLACK_BOT_TOKEN") @@ -88,6 +90,8 @@ def _load_config_from_root(root: Path) -> Config: slack_bot_token=slack_bot_token, slack_app_token=slack_app_token, slack_allowed_user_ids=slack_allowed_user_ids, + base_dir=base_dir, + config_dir=root, github_token=github_token, ) @@ -113,7 +117,7 @@ def _load_allowed_user_ids() -> list[str]: return [uid.strip() for uid in raw_value.split(",") if uid.strip()] -def _load_projects(path: Path) -> Dict[str, Project]: +def _load_projects(path: Path) -> Tuple[Dict[str, Project], Path]: try: data = yaml.safe_load(path.read_text(encoding="utf-8")) except FileNotFoundError as exc: @@ -170,7 +174,7 @@ def _load_projects(path: Path) -> Dict[str, Project]: ) if not projects: LOGGER.warning("No projects configured in %s", path) - return projects + return projects, base_dir def _load_agents(path: Path) -> Dict[str, Agent]: diff --git a/src/core/errors.py b/src/core/errors.py index 37efa28..2a159b6 100644 --- a/src/core/errors.py +++ b/src/core/errors.py @@ -35,3 +35,8 @@ class SlackError(RemoteCoderError): class GitHubError(RemoteCoderError): pass + + +class ProjectCreationError(RemoteCoderError): + """Raised when project creation fails.""" + pass diff --git a/src/core/project_creation.py b/src/core/project_creation.py new file mode 100644 index 0000000..4449e56 --- /dev/null +++ b/src/core/project_creation.py @@ -0,0 +1,310 @@ +"""Service for creating new projects with GitHub integration.""" + +from __future__ import annotations + +import asyncio +import logging +import re +import shutil +from dataclasses import dataclass +from pathlib import Path + +import yaml + +from .config import Config +from .errors import GitHubError, ProjectCreationError, ProjectNotFound +from .models import GitHubRepoConfig, Project +from ..github import GitHubManager + +LOGGER = logging.getLogger(__name__) + +# Valid project ID pattern: alphanumeric, hyphens, underscores +PROJECT_ID_PATTERN = re.compile(r"^[a-zA-Z0-9][a-zA-Z0-9_-]*$") + + +@dataclass +class ProjectCreationRequest: + """Configuration for creating a new project.""" + + project_id: str + channel_name: str + default_agent_id: str = "claude" + default_base_branch: str = "main" + + +class ProjectCreationService: + """Encapsulates project creation workflow.""" + + def __init__( + self, + config: Config, + github_manager: GitHubManager, + ) -> None: + self._config = config + self._github = github_manager + + def update_config(self, config: Config) -> None: + """Update the config reference.""" + self._config = config + + async def create_project(self, request: ProjectCreationRequest) -> Project: + """ + Create a new project with local directory and GitHub repo. + + Steps: + 1. Validate project ID and ensure project doesn't exist + 2. Create local directory + 3. Initialize git repository + 4. Create GitHub repository (private) + 5. Add initial commit (README.md) + 6. Push to GitHub + 7. Update projects.yaml + + Returns: + Created Project object + + Raises: + ProjectCreationError: If any step fails + """ + project_id = request.project_id + + # Validate project ID + self._validate_project_id(project_id) + + local_path = self._config.base_dir / project_id + + # Validate project doesn't exist + if local_path.exists(): + raise ProjectCreationError(f"Directory already exists: {local_path}") + + try: + self._config.get_project_by_channel(project_id) + raise ProjectCreationError(f"Project '{project_id}' is already configured") + except ProjectNotFound: + pass # Good, doesn't exist + + try: + # Get GitHub owner + github_owner = await self._get_github_owner() + + # Create local directory + local_path.mkdir(parents=True, exist_ok=False) + LOGGER.info("Created local directory: %s", local_path) + + # Initialize git + await self._init_git_repo(local_path, request.default_base_branch) + LOGGER.info("Initialized git repository") + + # Create GitHub repo + repo_full_name = await self._create_github_repo( + owner=github_owner, + repo_name=project_id, + description=f"Created from Slack channel #{request.channel_name}", + ) + LOGGER.info("Created GitHub repository: %s", repo_full_name) + + # Add initial commit + await self._create_initial_commit( + local_path, project_id, request.channel_name + ) + LOGGER.info("Created initial commit") + + # Push to GitHub + remote_url = f"git@github.com:{repo_full_name}.git" + await self._push_to_github( + local_path, remote_url, request.default_base_branch + ) + LOGGER.info("Pushed to GitHub") + + # Update projects.yaml + project = await self._add_to_config( + project_id=project_id, + channel_name=request.channel_name, + local_path=local_path, + github_owner=github_owner, + repo_name=project_id, + default_base_branch=request.default_base_branch, + default_agent_id=request.default_agent_id, + ) + LOGGER.info("Updated projects.yaml") + + return project + + except ProjectCreationError: + # Cleanup and re-raise + self._cleanup_failed_creation(local_path) + raise + except Exception as e: + # Cleanup on unexpected failure + self._cleanup_failed_creation(local_path) + raise ProjectCreationError( + f"Failed to create project '{project_id}': {e}" + ) from e + + def _validate_project_id(self, project_id: str) -> None: + """Validate project ID for safety and compatibility.""" + if not project_id: + raise ProjectCreationError("Project ID cannot be empty") + + if len(project_id) > 100: + raise ProjectCreationError("Project ID too long (max 100 characters)") + + # Prevent directory traversal + if ".." in project_id or "/" in project_id or "\\" in project_id: + raise ProjectCreationError( + "Project ID cannot contain path separators or '..'" + ) + + if not PROJECT_ID_PATTERN.match(project_id): + raise ProjectCreationError( + "Project ID must start with alphanumeric and contain only " + "alphanumeric characters, hyphens, or underscores" + ) + + async def _get_github_owner(self) -> str: + """Get the GitHub owner/username for creating repos.""" + if not self._github.is_configured(): + raise GitHubError("GitHub token is not configured") + + def _get_user_login() -> str: + user = self._github._client.get_user() + return user.login + + return await asyncio.to_thread(_get_user_login) + + async def _init_git_repo(self, path: Path, default_branch: str) -> None: + """Initialize a git repository.""" + await self._run_git(path, ["init"]) + await self._run_git(path, ["branch", "-M", default_branch]) + + async def _create_github_repo( + self, + owner: str, + repo_name: str, + description: str, + ) -> str: + """ + Create a private GitHub repository. + + Returns: + Full repository name (owner/repo) + """ + + def _create_repo() -> str: + user = self._github._client.get_user() + repo = user.create_repo( + name=repo_name, + description=description, + private=True, + auto_init=False, + ) + return repo.full_name + + try: + return await asyncio.to_thread(_create_repo) + except Exception as e: + raise GitHubError(f"Failed to create GitHub repository: {e}") from e + + async def _create_initial_commit( + self, + path: Path, + project_id: str, + channel_name: str, + ) -> None: + """Create initial README and commit.""" + readme = path / "README.md" + readme.write_text( + f"# {project_id}\n\nCreated from Slack channel #{channel_name}\n" + ) + await self._run_git(path, ["add", "README.md"]) + await self._run_git(path, ["commit", "-m", "Initial commit"]) + + async def _push_to_github( + self, + path: Path, + remote_url: str, + default_branch: str, + ) -> None: + """Add remote and push.""" + await self._run_git(path, ["remote", "add", "origin", remote_url]) + await self._run_git(path, ["push", "-u", "origin", default_branch]) + + async def _add_to_config( + self, + project_id: str, + channel_name: str, + local_path: Path, + github_owner: str, + repo_name: str, + default_base_branch: str, + default_agent_id: str, + ) -> Project: + """Add project entry to projects.yaml.""" + projects_yaml = self._config.config_dir / "projects.yaml" + + # Load existing + with open(projects_yaml, "r") as f: + data = yaml.safe_load(f) or {} + + if "projects" not in data: + data["projects"] = {} + + # Add new project - use relative path from base_dir + try: + relative_path = local_path.relative_to(self._config.base_dir) + except ValueError: + # If local_path is not relative to base_dir, use the full path + relative_path = local_path + + data["projects"][project_id] = { + "path": str(relative_path), + "default_agent": default_agent_id, + "github": { + "owner": github_owner, + "repo": repo_name, + "default_base_branch": default_base_branch, + }, + } + + # Write back + with open(projects_yaml, "w") as f: + yaml.dump(data, f, default_flow_style=False, sort_keys=False) + + # Create Project object + return Project( + id=project_id, + channel_name=channel_name, + path=local_path, + default_agent_id=default_agent_id, + github=GitHubRepoConfig( + owner=github_owner, + repo=repo_name, + default_base_branch=default_base_branch, + ), + ) + + async def _run_git(self, cwd: Path, args: list[str]) -> str: + """Run a git command asynchronously.""" + proc = await asyncio.create_subprocess_exec( + "git", + *args, + cwd=cwd, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) + stdout, stderr = await proc.communicate() + + if proc.returncode != 0: + error_msg = stderr.decode().strip() if stderr else "Unknown error" + raise ProjectCreationError(f"Git command failed: git {' '.join(args)}: {error_msg}") + + return stdout.decode().strip() if stdout else "" + + def _cleanup_failed_creation(self, path: Path) -> None: + """Clean up local directory on failure.""" + if path.exists(): + try: + shutil.rmtree(path) + LOGGER.info("Cleaned up failed creation: %s", path) + except Exception as e: + LOGGER.warning("Failed to cleanup %s: %s", path, e) diff --git a/src/core/router.py b/src/core/router.py index 7df0c7b..be64db1 100644 --- a/src/core/router.py +++ b/src/core/router.py @@ -18,6 +18,7 @@ from .commands.context import CommandContext from .commands.dispatcher import CommandDispatcher from .commands.maintenance import MaintenanceCommandHandler +from .commands.project_creation import ProjectCreationHandler from .commands.registry import CommandSpec from .commands.review import ReviewCommandHandler from .commands.session import SessionCommandHandler @@ -52,6 +53,11 @@ def __init__( self.active_runs: Dict[str, Dict[str, Any]] = {} self._interaction_classifier = InteractionClassifier() self._command_dispatcher = CommandDispatcher() + self._project_creation_handler = ProjectCreationHandler( + config=self._config, + github_manager=self._github_manager, + config_root=self._config_root, + ) self._git_workflow = GitWorkflowService( github_manager=self._github_manager, session_manager=self._session_manager, @@ -118,6 +124,7 @@ def _apply_new_config(self, new_config: Config) -> None: self._session_commands.update_config(new_config) self._catalog_commands.update_config(new_config) self._agent_runner.update_config(new_config) + self._project_creation_handler.update_config(new_config) if self._chat_adapter and hasattr(self._chat_adapter, "update_allowed_users"): try: @@ -135,10 +142,22 @@ async def handle_message(self, event: Dict[str, Any]) -> None: LOGGER.debug("Ignoring Slack event missing channel or thread") return + # Check if this is a response to a pending project creation prompt + was_handled, new_config = await self._project_creation_handler.handle_response( + channel_id, text, self._send_message + ) + if was_handled: + if new_config: + self._apply_new_config(new_config) + return + try: project = self._config.get_project_by_channel(channel_lookup) except ProjectNotFound: LOGGER.warning("No project mapping for channel %s", channel_lookup) + await self._project_creation_handler.handle_missing_project( + channel_id, channel_lookup, thread_ts, self._send_message + ) return session, created = self._get_or_create_session(project, channel_id, thread_ts) @@ -287,8 +306,12 @@ def _get_session_lock(self, session_key: str) -> asyncio.Lock: self._session_locks[session_key] = lock return lock - async def _send_message(self, channel: str, thread_ts: str, text: str) -> None: + async def _send_message( + self, channel: str, thread_ts: str, text: str + ) -> Optional[str]: if not self._chat_adapter: LOGGER.warning("Chat adapter not bound; dropping message: %s", text) - return - await self._chat_adapter.send_message(channel=channel, thread_ts=thread_ts, text=text) + return None + return await self._chat_adapter.send_message( + channel=channel, thread_ts=thread_ts, text=text + ) diff --git a/tests/commands/conftest.py b/tests/commands/conftest.py index 60365d2..779cb16 100644 --- a/tests/commands/conftest.py +++ b/tests/commands/conftest.py @@ -49,7 +49,7 @@ def test_project(tmp_path): @pytest.fixture -def test_config(): +def test_config(tmp_path): """Create a real Config with agents suitable for tests.""" agents = { "claude": Agent( @@ -74,6 +74,8 @@ def test_config(): slack_bot_token="bot-token", slack_app_token="app-token", slack_allowed_user_ids=["U123"], + base_dir=tmp_path / "projects", + config_dir=tmp_path / "config", github_token=None, ) return config diff --git a/tests/test_project_creation.py b/tests/test_project_creation.py new file mode 100644 index 0000000..2a38059 --- /dev/null +++ b/tests/test_project_creation.py @@ -0,0 +1,242 @@ +"""Tests for ProjectCreationService.""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from src.core.config import Config +from src.core.errors import GitHubError, ProjectCreationError, ProjectNotFound +from src.core.models import Agent, AgentType, GitHubRepoConfig, Project, WorkingDirMode +from src.core.project_creation import ProjectCreationRequest, ProjectCreationService + + +@pytest.fixture +def test_config(tmp_path): + """Create a test config with base_dir and config_dir.""" + base_dir = tmp_path / "projects" + base_dir.mkdir() + config_dir = tmp_path / "config" + config_dir.mkdir() + + # Create projects.yaml + projects_yaml = config_dir / "projects.yaml" + projects_yaml.write_text(f'base_dir: "{base_dir}"\nprojects: {{}}\n') + + return Config( + projects={}, + agents={ + "claude": Agent( + id="claude", + type=AgentType.CLAUDE, + command=["claude"], + working_dir_mode=WorkingDirMode.PROJECT, + ), + }, + slack_bot_token="bot-token", + slack_app_token="app-token", + slack_allowed_user_ids=["U123"], + base_dir=base_dir, + config_dir=config_dir, + github_token="test-token", + ) + + +@pytest.fixture +def mock_github_manager(): + """Create a mock GitHubManager.""" + manager = MagicMock() + manager.is_configured.return_value = True + manager._client = MagicMock() + + # Mock user + mock_user = MagicMock() + mock_user.login = "test-user" + + # Mock repo creation + mock_repo = MagicMock() + mock_repo.full_name = "test-user/test-project" + + mock_user.create_repo.return_value = mock_repo + manager._client.get_user.return_value = mock_user + + return manager + + +class TestProjectCreationService: + """Tests for ProjectCreationService.""" + + def test_validate_project_id_empty(self, test_config, mock_github_manager): + """Test that empty project ID raises error.""" + service = ProjectCreationService(test_config, mock_github_manager) + with pytest.raises(ProjectCreationError, match="cannot be empty"): + service._validate_project_id("") + + def test_validate_project_id_too_long(self, test_config, mock_github_manager): + """Test that overly long project ID raises error.""" + service = ProjectCreationService(test_config, mock_github_manager) + with pytest.raises(ProjectCreationError, match="too long"): + service._validate_project_id("a" * 101) + + def test_validate_project_id_path_traversal(self, test_config, mock_github_manager): + """Test that path traversal attempts are blocked.""" + service = ProjectCreationService(test_config, mock_github_manager) + with pytest.raises(ProjectCreationError, match="path separators"): + service._validate_project_id("../evil") + with pytest.raises(ProjectCreationError, match="path separators"): + service._validate_project_id("foo/bar") + with pytest.raises(ProjectCreationError, match="path separators"): + service._validate_project_id("foo\\bar") + + def test_validate_project_id_invalid_chars(self, test_config, mock_github_manager): + """Test that invalid characters are rejected.""" + service = ProjectCreationService(test_config, mock_github_manager) + with pytest.raises(ProjectCreationError, match="must start with alphanumeric"): + service._validate_project_id("-starts-with-dash") + with pytest.raises(ProjectCreationError, match="must start with alphanumeric"): + service._validate_project_id("_starts-with-underscore") + + def test_validate_project_id_valid(self, test_config, mock_github_manager): + """Test that valid project IDs pass validation.""" + service = ProjectCreationService(test_config, mock_github_manager) + # These should not raise + service._validate_project_id("my-project") + service._validate_project_id("my_project") + service._validate_project_id("MyProject123") + service._validate_project_id("a") + + @pytest.mark.asyncio + async def test_create_project_directory_exists( + self, test_config, mock_github_manager + ): + """Test that creating a project fails if directory exists.""" + service = ProjectCreationService(test_config, mock_github_manager) + + # Create the directory first + (test_config.base_dir / "existing-project").mkdir() + + request = ProjectCreationRequest( + project_id="existing-project", + channel_name="existing-project", + ) + + with pytest.raises(ProjectCreationError, match="Directory already exists"): + await service.create_project(request) + + @pytest.mark.asyncio + async def test_create_project_already_configured( + self, test_config, mock_github_manager + ): + """Test that creating an already configured project fails.""" + # Add project to config + test_config.projects["test-project"] = Project( + id="test-project", + channel_name="test-project", + path=test_config.base_dir / "test-project", + default_agent_id="claude", + ) + + service = ProjectCreationService(test_config, mock_github_manager) + request = ProjectCreationRequest( + project_id="test-project", + channel_name="test-project", + ) + + with pytest.raises(ProjectCreationError, match="already configured"): + await service.create_project(request) + + @pytest.mark.asyncio + async def test_create_project_github_not_configured(self, test_config): + """Test that creating a project fails if GitHub is not configured.""" + mock_github = MagicMock() + mock_github.is_configured.return_value = False + + service = ProjectCreationService(test_config, mock_github) + request = ProjectCreationRequest( + project_id="new-project", + channel_name="new-project", + ) + + with pytest.raises(ProjectCreationError, match="not configured"): + await service.create_project(request) + + @pytest.mark.asyncio + async def test_create_project_success(self, test_config, mock_github_manager): + """Test successful project creation.""" + service = ProjectCreationService(test_config, mock_github_manager) + + # Mock git commands to succeed + with patch.object(service, "_run_git", new_callable=AsyncMock) as mock_git: + mock_git.return_value = "" + + request = ProjectCreationRequest( + project_id="new-project", + channel_name="new-project", + default_agent_id="claude", + default_base_branch="main", + ) + + project = await service.create_project(request) + + assert project.id == "new-project" + assert project.channel_name == "new-project" + assert project.default_agent_id == "claude" + assert project.github is not None + assert project.github.owner == "test-user" + assert project.github.repo == "new-project" + assert project.github.default_base_branch == "main" + + # Verify local directory was created + assert (test_config.base_dir / "new-project").exists() + assert (test_config.base_dir / "new-project" / "README.md").exists() + + # Verify projects.yaml was updated + import yaml + + projects_yaml = test_config.config_dir / "projects.yaml" + with open(projects_yaml) as f: + data = yaml.safe_load(f) + + assert "new-project" in data["projects"] + assert data["projects"]["new-project"]["default_agent"] == "claude" + assert data["projects"]["new-project"]["github"]["owner"] == "test-user" + + @pytest.mark.asyncio + async def test_create_project_cleanup_on_failure( + self, test_config, mock_github_manager + ): + """Test that failed creation cleans up local directory.""" + service = ProjectCreationService(test_config, mock_github_manager) + + # Mock git init to succeed but push to fail + call_count = 0 + + async def mock_git(cwd, args): + nonlocal call_count + call_count += 1 + if "push" in args: + raise ProjectCreationError("Push failed") + return "" + + with patch.object(service, "_run_git", side_effect=mock_git): + request = ProjectCreationRequest( + project_id="failed-project", + channel_name="failed-project", + ) + + with pytest.raises(ProjectCreationError, match="Push failed"): + await service.create_project(request) + + # Verify local directory was cleaned up + assert not (test_config.base_dir / "failed-project").exists() + + def test_update_config(self, test_config, mock_github_manager): + """Test that update_config updates the internal config reference.""" + service = ProjectCreationService(test_config, mock_github_manager) + + new_config = MagicMock() + service.update_config(new_config) + + assert service._config is new_config diff --git a/tests/test_router_integration.py b/tests/test_router_integration.py index 984323f..84c9bfb 100644 --- a/tests/test_router_integration.py +++ b/tests/test_router_integration.py @@ -37,9 +37,13 @@ class DummyChatAdapter: def __init__(self) -> None: self.messages: list[Dict[str, str]] = [] + self._msg_counter = 0 - async def send_message(self, channel: str, thread_ts: str, text: str) -> None: + async def send_message(self, channel: str, thread_ts: str, text: str) -> str: + self._msg_counter += 1 + msg_ts = f"{thread_ts}.{self._msg_counter}" self.messages.append({"channel": channel, "thread_ts": thread_ts, "text": text}) + return msg_ts @pytest.fixture @@ -68,6 +72,8 @@ def router_setup(tmp_path): slack_bot_token="x", slack_app_token="y", slack_allowed_user_ids=["U123"], + base_dir=tmp_path / "projects", + config_dir=tmp_path / "config", github_token=None, ) github_manager = StubGitHubManager() diff --git a/tests/test_router_reactions.py b/tests/test_router_reactions.py new file mode 100644 index 0000000..dba87ad --- /dev/null +++ b/tests/test_router_reactions.py @@ -0,0 +1,451 @@ +"""Tests for Router project creation flow with Y/N text responses.""" + +from __future__ import annotations + +from typing import Any, Dict, Optional +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from src.core.config import Config +from src.core.models import Agent, AgentType, GitHubRepoConfig, Project, WorkingDirMode +from src.core.commands.project_creation import PendingProjectCreation +from src.core.router import Router +from src.core.conversation.session_manager import SessionManager + + +class StubGitHubManager: + """Minimal GitHub manager stub for tests.""" + + def __init__(self) -> None: + self.token = None + self._client = MagicMock() + + def update_token(self, token: str | None) -> None: + self.token = token + + def is_configured(self) -> bool: + return True + + async def get_unresolved_comments(self, *args: Any, **kwargs: Any): + return [] + + async def ensure_pull_request(self, *args: Any, **kwargs: Any): + raise AssertionError("PR publishing should not occur in these tests") + + +class DummyChatAdapter: + """Captures Slack messages emitted by the router.""" + + def __init__(self) -> None: + self.messages: list[Dict[str, str]] = [] + self._msg_counter = 0 + + async def send_message( + self, channel: str, thread_ts: str, text: str + ) -> Optional[str]: + self._msg_counter += 1 + msg_ts = f"{thread_ts}.{self._msg_counter}" + self.messages.append( + {"channel": channel, "thread_ts": thread_ts, "text": text, "ts": msg_ts} + ) + return msg_ts + + +@pytest.fixture +def router_setup(tmp_path): + """Set up a router for testing with no projects configured.""" + session_manager = SessionManager(history_limit=20) + base_dir = tmp_path / "projects" + base_dir.mkdir() + config_dir = tmp_path / "config" + config_dir.mkdir() + + # Create projects.yaml + projects_yaml = config_dir / "projects.yaml" + projects_yaml.write_text(f'base_dir: "{base_dir}"\nprojects: {{}}\n') + + # Create agents.yaml (needed for config reload) + agents_yaml = config_dir / "agents.yaml" + agents_yaml.write_text("""agents: + claude: + type: claude + command: ["echo"] + working_dir_mode: project + models: + default: sonnet + available: ["sonnet"] +""") + + # Create .env file (needed for config reload) + env_file = config_dir / ".env" + env_file.write_text("""SLACK_BOT_TOKEN=x +SLACK_APP_TOKEN=y +SLACK_ALLOWED_USER_IDS=U123 +GITHUB_TOKEN=test-token +""") + + agent = Agent( + id="claude", + type=AgentType.CLAUDE, + command=["echo"], + working_dir_mode=WorkingDirMode.PROJECT, + models={"default": "sonnet", "available": ["sonnet"]}, + ) + config = Config( + projects={}, # No projects configured + agents={agent.id: agent}, + slack_bot_token="x", + slack_app_token="y", + slack_allowed_user_ids=["U123"], + base_dir=base_dir, + config_dir=config_dir, + github_token="test-token", + ) + github_manager = StubGitHubManager() + router = Router(session_manager, config, github_manager, config_root=config_dir) + router._git_workflow.setup_session_branch = AsyncMock(return_value=None) + router._git_workflow.maybe_publish_code_changes = AsyncMock(return_value=None) + router._agent_runner.run = AsyncMock() + + adapter = DummyChatAdapter() + router.bind_adapter(adapter) + return router, adapter, tmp_path + + +class TestPendingProjectCreation: + """Tests for PendingProjectCreation dataclass.""" + + def test_pending_project_creation_fields(self): + """Test PendingProjectCreation has expected fields.""" + pending = PendingProjectCreation( + channel_id="C123", + channel_name="my-project", + thread_ts="111.000", + created_at=1234567890.0, + ) + assert pending.channel_id == "C123" + assert pending.channel_name == "my-project" + assert pending.thread_ts == "111.000" + assert pending.created_at == 1234567890.0 + + +class TestRouterMissingProject: + """Tests for handling messages to channels without projects.""" + + @pytest.mark.asyncio + async def test_missing_project_sends_prompt(self, router_setup): + """Test that message to unknown channel sends creation prompt.""" + router, adapter, _ = router_setup + + event = { + "channel": "C123", + "channel_name": "new-idea", + "text": "Hello", + "ts": "111.222", + } + + await router.handle_message(event) + + # Should have sent the prompt message + assert len(adapter.messages) == 1 + assert "new-idea" in adapter.messages[0]["text"] + assert "Reply with **Y**" in adapter.messages[0]["text"] + + @pytest.mark.asyncio + async def test_missing_project_tracks_pending(self, router_setup): + """Test that pending project creation is tracked.""" + router, adapter, _ = router_setup + + event = { + "channel": "C123", + "channel_name": "new-idea", + "text": "Hello", + "ts": "111.222", + } + + await router.handle_message(event) + + # Should be tracking the pending creation by channel_id + handler = router._project_creation_handler + assert "C123" in handler._pending_projects + pending = handler._pending_projects["C123"] + assert pending.channel_id == "C123" + assert pending.channel_name == "new-idea" + + +class TestRouterYesNoHandling: + """Tests for Y/N response handling in Router.""" + + @pytest.mark.asyncio + async def test_yes_response_creates_project(self, router_setup): + """Test that 'Y' response triggers project creation.""" + router, adapter, tmp_path = router_setup + + # First, send a message to trigger the prompt + event1 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "Hello", + "ts": "111.222", + } + await router.handle_message(event1) + + # Now send 'Y' response + with patch.object( + router._project_creation_handler._project_creator, "create_project", new_callable=AsyncMock + ) as mock_create: + mock_create.return_value = Project( + id="new-idea", + channel_name="new-idea", + path=tmp_path / "projects" / "new-idea", + default_agent_id="claude", + github=GitHubRepoConfig( + owner="test-user", + repo="new-idea", + default_base_branch="main", + ), + ) + + event2 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "Y", + "ts": "111.333", + } + await router.handle_message(event2) + + # Should have called create_project + mock_create.assert_called_once() + + # Should have sent "Creating..." and success messages + assert any("Creating project" in msg["text"] for msg in adapter.messages) + assert any("I've set up" in msg["text"] for msg in adapter.messages) + + # Pending should be cleaned up + assert "C123" not in router._project_creation_handler._pending_projects + + @pytest.mark.asyncio + async def test_yes_lowercase_works(self, router_setup): + """Test that lowercase 'y' also works.""" + router, adapter, tmp_path = router_setup + + # Trigger the prompt + event1 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "Hello", + "ts": "111.222", + } + await router.handle_message(event1) + + with patch.object( + router._project_creation_handler._project_creator, "create_project", new_callable=AsyncMock + ) as mock_create: + mock_create.return_value = Project( + id="new-idea", + channel_name="new-idea", + path=tmp_path / "projects" / "new-idea", + default_agent_id="claude", + ) + + event2 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "y", + "ts": "111.333", + } + await router.handle_message(event2) + + mock_create.assert_called_once() + + @pytest.mark.asyncio + async def test_yes_full_word_works(self, router_setup): + """Test that 'yes' also works.""" + router, adapter, tmp_path = router_setup + + # Trigger the prompt + event1 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "Hello", + "ts": "111.222", + } + await router.handle_message(event1) + + with patch.object( + router._project_creation_handler._project_creator, "create_project", new_callable=AsyncMock + ) as mock_create: + mock_create.return_value = Project( + id="new-idea", + channel_name="new-idea", + path=tmp_path / "projects" / "new-idea", + default_agent_id="claude", + ) + + event2 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "yes", + "ts": "111.333", + } + await router.handle_message(event2) + + mock_create.assert_called_once() + + @pytest.mark.asyncio + async def test_no_response_rejects(self, router_setup): + """Test that 'N' response sends rejection message.""" + router, adapter, _ = router_setup + + # Trigger the prompt + event1 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "Hello", + "ts": "111.222", + } + await router.handle_message(event1) + + # Send 'N' response + event2 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "N", + "ts": "111.333", + } + await router.handle_message(event2) + + # Should have sent rejection message + assert any("rename the channel" in msg["text"] for msg in adapter.messages) + + # Pending should be cleaned up + assert "C123" not in router._project_creation_handler._pending_projects + + @pytest.mark.asyncio + async def test_no_lowercase_works(self, router_setup): + """Test that lowercase 'n' also works.""" + router, adapter, _ = router_setup + + event1 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "Hello", + "ts": "111.222", + } + await router.handle_message(event1) + + event2 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "n", + "ts": "111.333", + } + await router.handle_message(event2) + + assert any("rename the channel" in msg["text"] for msg in adapter.messages) + assert "C123" not in router._project_creation_handler._pending_projects + + @pytest.mark.asyncio + async def test_invalid_response_reminds_user(self, router_setup): + """Test that invalid responses remind user to use Y/N.""" + router, adapter, _ = router_setup + + # Trigger the prompt + event1 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "Hello", + "ts": "111.222", + } + await router.handle_message(event1) + + # Send invalid response + event2 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "maybe", + "ts": "111.333", + } + await router.handle_message(event2) + + # Should have sent reminder message + assert any("Please reply with **Y**" in msg["text"] for msg in adapter.messages) + + # Pending should still be tracked + assert "C123" in router._project_creation_handler._pending_projects + + @pytest.mark.asyncio + async def test_creation_failure_sends_error(self, router_setup): + """Test that failed project creation sends error message.""" + router, adapter, _ = router_setup + + # Trigger the prompt + event1 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "Hello", + "ts": "111.222", + } + await router.handle_message(event1) + + # Mock creation to fail + with patch.object( + router._project_creation_handler._project_creator, "create_project", new_callable=AsyncMock + ) as mock_create: + mock_create.side_effect = Exception("GitHub API error") + + event2 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "Y", + "ts": "111.333", + } + await router.handle_message(event2) + + # Should have sent error message + assert any("couldn't create the project" in msg["text"] for msg in adapter.messages) + + # Pending should be cleaned up even on failure + assert "C123" not in router._project_creation_handler._pending_projects + + +class TestProjectCreationHandler: + """Tests for ProjectCreationHandler directly.""" + + def test_pending_project_tracking(self, router_setup): + """Test that pending projects are tracked correctly.""" + router, _, _ = router_setup + handler = router._project_creation_handler + + # No pending projects initially + assert "C123" not in handler._pending_projects + + # Manually add a pending project + handler._pending_projects["C123"] = PendingProjectCreation( + channel_id="C123", + channel_name="test", + thread_ts="111.000", + created_at=12345.0, + ) + + assert "C123" in handler._pending_projects + assert "C456" not in handler._pending_projects + + def test_pending_project_fields(self, router_setup): + """Test PendingProjectCreation fields.""" + router, _, _ = router_setup + handler = router._project_creation_handler + + handler._pending_projects["C123"] = PendingProjectCreation( + channel_id="C123", + channel_name="my-project", + thread_ts="111.000", + created_at=12345.0, + ) + + pending = handler._pending_projects["C123"] + assert pending.channel_id == "C123" + assert pending.channel_name == "my-project" + assert pending.thread_ts == "111.000" + assert pending.created_at == 12345.0 From b90c5496b5d8b9718ab0900408ef36e1e12d8c44 Mon Sep 17 00:00:00 2001 From: PeterShin23 Date: Wed, 17 Dec 2025 23:41:27 -0500 Subject: [PATCH 2/9] handling github repo creation issues --- src/core/errors.py | 10 ++ src/core/project_creation.py | 179 +++++++++++++++++++---------- tests/test_project_creation.py | 198 ++++++++++++++++++++++++++------- 3 files changed, 289 insertions(+), 98 deletions(-) diff --git a/src/core/errors.py b/src/core/errors.py index 2a159b6..1b8a367 100644 --- a/src/core/errors.py +++ b/src/core/errors.py @@ -40,3 +40,13 @@ class GitHubError(RemoteCoderError): class ProjectCreationError(RemoteCoderError): """Raised when project creation fails.""" pass + + +class RepoExistsError(ProjectCreationError): + """Raised when GitHub repo name already exists.""" + pass + + +class LocalDirNotGitRepoError(ProjectCreationError): + """Raised when local directory exists but is not a git repository.""" + pass diff --git a/src/core/project_creation.py b/src/core/project_creation.py index 4449e56..c05ce9d 100644 --- a/src/core/project_creation.py +++ b/src/core/project_creation.py @@ -12,14 +12,20 @@ import yaml from .config import Config -from .errors import GitHubError, ProjectCreationError, ProjectNotFound +from .errors import ( + GitHubError, + LocalDirNotGitRepoError, + ProjectCreationError, + ProjectNotFound, + RepoExistsError, +) from .models import GitHubRepoConfig, Project from ..github import GitHubManager LOGGER = logging.getLogger(__name__) -# Valid project ID pattern: alphanumeric, hyphens, underscores -PROJECT_ID_PATTERN = re.compile(r"^[a-zA-Z0-9][a-zA-Z0-9_-]*$") +# Pattern to strip invalid characters from repo names (keep only letters, numbers, dashes) +REPO_NAME_INVALID_CHARS = re.compile(r"[^a-zA-Z0-9-]") @dataclass @@ -51,78 +57,133 @@ async def create_project(self, request: ProjectCreationRequest) -> Project: """ Create a new project with local directory and GitHub repo. - Steps: - 1. Validate project ID and ensure project doesn't exist - 2. Create local directory - 3. Initialize git repository - 4. Create GitHub repository (private) - 5. Add initial commit (README.md) - 6. Push to GitHub - 7. Update projects.yaml + Flow: + 1. Sanitize channel name to get repo name + 2. Check if local directory exists: + a. EXISTS + is git repo → just add to config + b. EXISTS + not git repo → raise LocalDirNotGitRepoError + c. DOESN'T EXIST → create dir, init git, create GitHub repo, push + 3. Update projects.yaml Returns: Created Project object Raises: ProjectCreationError: If any step fails + RepoExistsError: If GitHub repo name already exists + LocalDirNotGitRepoError: If local dir exists but isn't a git repo """ - project_id = request.project_id + repo_name = self._sanitize_repo_name(request.channel_name) + LOGGER.info("Sanitized channel '%s' to repo name '%s'", request.channel_name, repo_name) - # Validate project ID - self._validate_project_id(project_id) + local_path = self._config.base_dir / repo_name - local_path = self._config.base_dir / project_id + try: + self._config.get_project_by_channel(request.channel_name) + raise ProjectCreationError(f"Project '{request.channel_name}' is already configured") + except ProjectNotFound: + pass - # Validate project doesn't exist if local_path.exists(): - raise ProjectCreationError(f"Directory already exists: {local_path}") + return await self._handle_existing_directory( + local_path=local_path, + repo_name=repo_name, + request=request, + ) + else: + return await self._create_new_project( + local_path=local_path, + repo_name=repo_name, + request=request, + ) - try: - self._config.get_project_by_channel(project_id) - raise ProjectCreationError(f"Project '{project_id}' is already configured") - except ProjectNotFound: - pass # Good, doesn't exist + async def _handle_existing_directory( + self, + local_path: Path, + repo_name: str, + request: ProjectCreationRequest, + ) -> Project: + """Handle the case where local directory already exists.""" + git_dir = local_path / ".git" + + if git_dir.exists() and git_dir.is_dir(): + LOGGER.info("Found existing git repo at %s, adding to config", local_path) + + # Try to parse owner from remote URL + github_owner = None + try: + remote_url = await self._run_git(local_path, ["remote", "get-url", "origin"]) + if "github.com" in remote_url: + if remote_url.startswith("git@"): + parts = remote_url.split(":")[-1].replace(".git", "").split("/") + else: + parts = remote_url.replace(".git", "").split("/")[-2:] + if len(parts) >= 2: + github_owner = parts[0] + except ProjectCreationError: + pass + + if not github_owner: + try: + github_owner = await self._get_github_owner() + except GitHubError: + github_owner = "unknown" + + return await self._add_to_config( + project_id=repo_name, + channel_name=request.channel_name, + local_path=local_path, + github_owner=github_owner, + repo_name=repo_name, + default_base_branch=request.default_base_branch, + default_agent_id=request.default_agent_id, + ) + else: + raise LocalDirNotGitRepoError( + f"Directory '{local_path}' exists but isn't a git repository. " + "This isn't supported yet, sorry :(" + ) + async def _create_new_project( + self, + local_path: Path, + repo_name: str, + request: ProjectCreationRequest, + ) -> Project: + """Create a completely new project with GitHub repo.""" try: - # Get GitHub owner github_owner = await self._get_github_owner() - # Create local directory local_path.mkdir(parents=True, exist_ok=False) LOGGER.info("Created local directory: %s", local_path) - # Initialize git await self._init_git_repo(local_path, request.default_base_branch) LOGGER.info("Initialized git repository") - # Create GitHub repo repo_full_name = await self._create_github_repo( owner=github_owner, - repo_name=project_id, + repo_name=repo_name, description=f"Created from Slack channel #{request.channel_name}", ) LOGGER.info("Created GitHub repository: %s", repo_full_name) - # Add initial commit await self._create_initial_commit( - local_path, project_id, request.channel_name + local_path, repo_name, request.channel_name ) LOGGER.info("Created initial commit") - # Push to GitHub remote_url = f"git@github.com:{repo_full_name}.git" await self._push_to_github( local_path, remote_url, request.default_base_branch ) LOGGER.info("Pushed to GitHub") - # Update projects.yaml project = await self._add_to_config( - project_id=project_id, + project_id=repo_name, channel_name=request.channel_name, local_path=local_path, github_owner=github_owner, - repo_name=project_id, + repo_name=repo_name, default_base_branch=request.default_base_branch, default_agent_id=request.default_agent_id, ) @@ -130,36 +191,32 @@ async def create_project(self, request: ProjectCreationRequest) -> Project: return project - except ProjectCreationError: - # Cleanup and re-raise + except (ProjectCreationError, RepoExistsError, LocalDirNotGitRepoError, GitHubError): self._cleanup_failed_creation(local_path) raise except Exception as e: - # Cleanup on unexpected failure self._cleanup_failed_creation(local_path) raise ProjectCreationError( - f"Failed to create project '{project_id}': {e}" + f"Something unexpected happened while creating '{repo_name}'. " + f"You'll need to check this when you're home. Sorry :( ({e})" ) from e - def _validate_project_id(self, project_id: str) -> None: - """Validate project ID for safety and compatibility.""" - if not project_id: - raise ProjectCreationError("Project ID cannot be empty") + def _sanitize_repo_name(self, channel_name: str) -> str: + """ + Sanitize channel name to a valid GitHub repo name. - if len(project_id) > 100: - raise ProjectCreationError("Project ID too long (max 100 characters)") + Keeps only letters, numbers, and dashes. + Strips leading/trailing dashes. + """ + sanitized = REPO_NAME_INVALID_CHARS.sub("", channel_name).strip("-") - # Prevent directory traversal - if ".." in project_id or "/" in project_id or "\\" in project_id: + if not sanitized: raise ProjectCreationError( - "Project ID cannot contain path separators or '..'" + f"Channel name '{channel_name}' results in an empty repo name after sanitization. " + "Please rename the channel to include letters or numbers." ) - if not PROJECT_ID_PATTERN.match(project_id): - raise ProjectCreationError( - "Project ID must start with alphanumeric and contain only " - "alphanumeric characters, hyphens, or underscores" - ) + return sanitized async def _get_github_owner(self) -> str: """Get the GitHub owner/username for creating repos.""" @@ -188,6 +245,10 @@ async def _create_github_repo( Returns: Full repository name (owner/repo) + + Raises: + RepoExistsError: If the repo name already exists + ProjectCreationError: For other GitHub API errors """ def _create_repo() -> str: @@ -203,7 +264,16 @@ def _create_repo() -> str: try: return await asyncio.to_thread(_create_repo) except Exception as e: - raise GitHubError(f"Failed to create GitHub repository: {e}") from e + error_str = str(e).lower() + if "already exists" in error_str: + raise RepoExistsError( + f"A GitHub repository named '{repo_name}' already exists. " + "Please rename the Slack channel and try again." + ) from e + raise ProjectCreationError( + f"Something unexpected happened while creating the GitHub repository. " + f"You'll need to check this when you're home. Sorry :( ({e})" + ) from e async def _create_initial_commit( self, @@ -242,18 +312,15 @@ async def _add_to_config( """Add project entry to projects.yaml.""" projects_yaml = self._config.config_dir / "projects.yaml" - # Load existing with open(projects_yaml, "r") as f: data = yaml.safe_load(f) or {} if "projects" not in data: data["projects"] = {} - # Add new project - use relative path from base_dir try: relative_path = local_path.relative_to(self._config.base_dir) except ValueError: - # If local_path is not relative to base_dir, use the full path relative_path = local_path data["projects"][project_id] = { @@ -266,11 +333,9 @@ async def _add_to_config( }, } - # Write back with open(projects_yaml, "w") as f: yaml.dump(data, f, default_flow_style=False, sort_keys=False) - # Create Project object return Project( id=project_id, channel_name=channel_name, diff --git a/tests/test_project_creation.py b/tests/test_project_creation.py index 2a38059..5ed0cad 100644 --- a/tests/test_project_creation.py +++ b/tests/test_project_creation.py @@ -8,7 +8,13 @@ import pytest from src.core.config import Config -from src.core.errors import GitHubError, ProjectCreationError, ProjectNotFound +from src.core.errors import ( + GitHubError, + LocalDirNotGitRepoError, + ProjectCreationError, + ProjectNotFound, + RepoExistsError, +) from src.core.models import Agent, AgentType, GitHubRepoConfig, Project, WorkingDirMode from src.core.project_creation import ProjectCreationRequest, ProjectCreationService @@ -65,66 +71,147 @@ def mock_github_manager(): return manager -class TestProjectCreationService: - """Tests for ProjectCreationService.""" +class TestSanitizeRepoName: + """Tests for _sanitize_repo_name method.""" - def test_validate_project_id_empty(self, test_config, mock_github_manager): - """Test that empty project ID raises error.""" + def test_sanitize_keeps_letters_numbers_dashes(self, test_config, mock_github_manager): + """Test that letters, numbers, and dashes are preserved.""" service = ProjectCreationService(test_config, mock_github_manager) - with pytest.raises(ProjectCreationError, match="cannot be empty"): - service._validate_project_id("") + assert service._sanitize_repo_name("my-project-123") == "my-project-123" - def test_validate_project_id_too_long(self, test_config, mock_github_manager): - """Test that overly long project ID raises error.""" + def test_sanitize_removes_underscores(self, test_config, mock_github_manager): + """Test that underscores are removed.""" service = ProjectCreationService(test_config, mock_github_manager) - with pytest.raises(ProjectCreationError, match="too long"): - service._validate_project_id("a" * 101) + assert service._sanitize_repo_name("my_project") == "myproject" - def test_validate_project_id_path_traversal(self, test_config, mock_github_manager): - """Test that path traversal attempts are blocked.""" + def test_sanitize_removes_special_chars(self, test_config, mock_github_manager): + """Test that special characters are removed.""" service = ProjectCreationService(test_config, mock_github_manager) - with pytest.raises(ProjectCreationError, match="path separators"): - service._validate_project_id("../evil") - with pytest.raises(ProjectCreationError, match="path separators"): - service._validate_project_id("foo/bar") - with pytest.raises(ProjectCreationError, match="path separators"): - service._validate_project_id("foo\\bar") - - def test_validate_project_id_invalid_chars(self, test_config, mock_github_manager): - """Test that invalid characters are rejected.""" + assert service._sanitize_repo_name("my.project!@#$%") == "myproject" + + def test_sanitize_strips_leading_trailing_dashes(self, test_config, mock_github_manager): + """Test that leading/trailing dashes are stripped.""" service = ProjectCreationService(test_config, mock_github_manager) - with pytest.raises(ProjectCreationError, match="must start with alphanumeric"): - service._validate_project_id("-starts-with-dash") - with pytest.raises(ProjectCreationError, match="must start with alphanumeric"): - service._validate_project_id("_starts-with-underscore") + assert service._sanitize_repo_name("-my-project-") == "my-project" + assert service._sanitize_repo_name("---test---") == "test" - def test_validate_project_id_valid(self, test_config, mock_github_manager): - """Test that valid project IDs pass validation.""" + def test_sanitize_empty_result_raises(self, test_config, mock_github_manager): + """Test that empty result after sanitization raises error.""" service = ProjectCreationService(test_config, mock_github_manager) - # These should not raise - service._validate_project_id("my-project") - service._validate_project_id("my_project") - service._validate_project_id("MyProject123") - service._validate_project_id("a") + with pytest.raises(ProjectCreationError, match="empty repo name"): + service._sanitize_repo_name("___") + with pytest.raises(ProjectCreationError, match="empty repo name"): + service._sanitize_repo_name("---") + with pytest.raises(ProjectCreationError, match="empty repo name"): + service._sanitize_repo_name("!@#$%") + + +class TestExistingDirectoryHandling: + """Tests for handling existing directories.""" @pytest.mark.asyncio - async def test_create_project_directory_exists( + async def test_existing_git_repo_adds_to_config( self, test_config, mock_github_manager ): - """Test that creating a project fails if directory exists.""" + """Test that existing git repo is just added to config.""" service = ProjectCreationService(test_config, mock_github_manager) - # Create the directory first - (test_config.base_dir / "existing-project").mkdir() + # Create existing git repo + repo_path = test_config.base_dir / "existing-repo" + repo_path.mkdir() + (repo_path / ".git").mkdir() + + # Mock _run_git to simulate remote URL fetch + async def mock_git(cwd, args): + if "get-url" in args: + return "git@github.com:existing-owner/existing-repo.git" + return "" + + with patch.object(service, "_run_git", side_effect=mock_git): + request = ProjectCreationRequest( + project_id="existing-repo", + channel_name="existing-repo", + ) + + project = await service.create_project(request) + + assert project.id == "existing-repo" + assert project.github.owner == "existing-owner" + + @pytest.mark.asyncio + async def test_existing_dir_not_git_repo_raises( + self, test_config, mock_github_manager + ): + """Test that existing non-git directory raises LocalDirNotGitRepoError.""" + service = ProjectCreationService(test_config, mock_github_manager) + + # Create existing directory without .git + (test_config.base_dir / "not-a-repo").mkdir() request = ProjectCreationRequest( - project_id="existing-project", - channel_name="existing-project", + project_id="not-a-repo", + channel_name="not-a-repo", ) - with pytest.raises(ProjectCreationError, match="Directory already exists"): + with pytest.raises(LocalDirNotGitRepoError, match="isn't a git repository"): await service.create_project(request) + +class TestGitHubRepoCreation: + """Tests for GitHub repository creation.""" + + @pytest.mark.asyncio + async def test_github_repo_exists_raises_repo_exists_error( + self, test_config, mock_github_manager + ): + """Test that 'repo exists' error raises RepoExistsError.""" + service = ProjectCreationService(test_config, mock_github_manager) + + # Make create_repo raise "already exists" error + mock_github_manager._client.get_user().create_repo.side_effect = Exception( + "Repository 'test' already exists on this account" + ) + + # Mock git commands to succeed + with patch.object(service, "_run_git", new_callable=AsyncMock) as mock_git: + mock_git.return_value = "" + + request = ProjectCreationRequest( + project_id="test-project", + channel_name="test-project", + ) + + with pytest.raises(RepoExistsError, match="already exists"): + await service.create_project(request) + + @pytest.mark.asyncio + async def test_github_other_error_raises_project_creation_error( + self, test_config, mock_github_manager + ): + """Test that other GitHub errors raise ProjectCreationError.""" + service = ProjectCreationService(test_config, mock_github_manager) + + # Make create_repo raise a different error + mock_github_manager._client.get_user().create_repo.side_effect = Exception( + "Rate limit exceeded" + ) + + # Mock git commands to succeed + with patch.object(service, "_run_git", new_callable=AsyncMock) as mock_git: + mock_git.return_value = "" + + request = ProjectCreationRequest( + project_id="test-project", + channel_name="test-project", + ) + + with pytest.raises(ProjectCreationError, match="Something unexpected"): + await service.create_project(request) + + +class TestProjectCreationService: + """Tests for ProjectCreationService.""" + @pytest.mark.asyncio async def test_create_project_already_configured( self, test_config, mock_github_manager @@ -159,7 +246,7 @@ async def test_create_project_github_not_configured(self, test_config): channel_name="new-project", ) - with pytest.raises(ProjectCreationError, match="not configured"): + with pytest.raises(GitHubError, match="not configured"): await service.create_project(request) @pytest.mark.asyncio @@ -203,6 +290,35 @@ async def test_create_project_success(self, test_config, mock_github_manager): assert data["projects"]["new-project"]["default_agent"] == "claude" assert data["projects"]["new-project"]["github"]["owner"] == "test-user" + @pytest.mark.asyncio + async def test_create_project_sanitizes_channel_name( + self, test_config, mock_github_manager + ): + """Test that channel name with special chars is sanitized.""" + service = ProjectCreationService(test_config, mock_github_manager) + + # Update mock to return correct repo name + mock_github_manager._client.get_user().create_repo.return_value.full_name = ( + "test-user/my-project" + ) + + # Mock git commands to succeed + with patch.object(service, "_run_git", new_callable=AsyncMock) as mock_git: + mock_git.return_value = "" + + request = ProjectCreationRequest( + project_id="my_project!@#", # Has special chars + channel_name="my_project!@#", + default_agent_id="claude", + default_base_branch="main", + ) + + project = await service.create_project(request) + + # Should be sanitized to "myproject" + assert project.id == "myproject" + assert (test_config.base_dir / "myproject").exists() + @pytest.mark.asyncio async def test_create_project_cleanup_on_failure( self, test_config, mock_github_manager From dd5f83ccd40771fd96c8d49d13cf43e42e17f0cf Mon Sep 17 00:00:00 2001 From: PeterShin23 Date: Thu, 18 Dec 2025 00:14:49 -0500 Subject: [PATCH 3/9] added agent selection on project add --- src/core/commands/project_creation.py | 160 ++++++++++++++++++++++---- src/core/config.py | 3 + src/core/models.py | 1 + src/core/project_creation.py | 12 +- tests/test_router_reactions.py | 144 +++++++++++++---------- 5 files changed, 233 insertions(+), 87 deletions(-) diff --git a/src/core/commands/project_creation.py b/src/core/commands/project_creation.py index 5624d86..be6dced 100644 --- a/src/core/commands/project_creation.py +++ b/src/core/commands/project_creation.py @@ -4,8 +4,8 @@ import logging import time -from dataclasses import dataclass -from typing import Awaitable, Callable, Dict, Optional +from dataclasses import dataclass, field +from typing import Awaitable, Callable, Dict, List, Optional from ..config import Config, load_config from ..project_creation import ProjectCreationRequest, ProjectCreationService @@ -16,6 +16,11 @@ # Type alias for the message sender callback SendMessage = Callable[[str, str, str], Awaitable[Optional[str]]] +# States for the project creation flow +STATE_AWAITING_CONFIRMATION = "awaiting_confirmation" +STATE_AWAITING_AGENT = "awaiting_agent" +STATE_AWAITING_MODEL = "awaiting_model" + @dataclass class PendingProjectCreation: @@ -25,6 +30,10 @@ class PendingProjectCreation: channel_name: str thread_ts: str created_at: float + state: str = STATE_AWAITING_CONFIRMATION + selected_agent_id: Optional[str] = None + agent_options: List[str] = field(default_factory=list) + model_options: List[str] = field(default_factory=list) class ProjectCreationHandler: @@ -70,8 +79,8 @@ async def handle_missing_project( await send_message( channel_id, thread_ts, - f"I couldn't find a project named **{channel_name}**. Is this a new idea?!?!\n\n" - "Reply with **Y** to create it, or **N** to cancel.", + f"I couldn't find a project named `{channel_name}`. Is this a new idea?!?!\n\n" + "Reply with \"Y\" to create it, or \"N\" to cancel.", ) # Track pending creation @@ -89,17 +98,10 @@ async def handle_response( send_message: SendMessage, ) -> tuple[bool, Optional[Config]]: """ - Handle a potential Y/N response to a pending project creation prompt. - - Args: - channel_id: The channel ID - text: The message text - send_message: Callback to send messages + Handle a response to a pending project creation prompt. Returns: Tuple of (was_handled, new_config_if_created) - - was_handled: True if this was a response to a pending prompt - - new_config_if_created: New Config if project was created, None otherwise """ pending = self._pending_projects.get(channel_id) if not pending: @@ -107,56 +109,166 @@ async def handle_response( response = text.strip().lower() + if pending.state == STATE_AWAITING_CONFIRMATION: + return await self._handle_confirmation_response(pending, response, send_message) + elif pending.state == STATE_AWAITING_AGENT: + return await self._handle_agent_response(pending, response, send_message) + elif pending.state == STATE_AWAITING_MODEL: + return await self._handle_model_response(pending, response, send_message) + + return False, None + + async def _handle_confirmation_response( + self, + pending: PendingProjectCreation, + response: str, + send_message: SendMessage, + ) -> tuple[bool, Optional[Config]]: + """Handle Y/N confirmation response.""" if response in ("y", "yes"): - del self._pending_projects[channel_id] - new_config = await self._handle_approval(pending, send_message) - return True, new_config + await self._show_agent_options(pending, send_message) + return True, None elif response in ("n", "no"): - del self._pending_projects[channel_id] + del self._pending_projects[pending.channel_id] await self._handle_rejection(pending, send_message) return True, None else: - # Not a y/n response, remind them await send_message( pending.channel_id, pending.thread_ts, - f"Please reply with **Y** to create project **{pending.channel_name}**, or **N** to cancel.", + f"Please reply with \"Y\" to create project `{pending.channel_name}`, or \"N\" to cancel.", ) return True, None - async def _handle_approval( + async def _handle_agent_response( + self, + pending: PendingProjectCreation, + response: str, + send_message: SendMessage, + ) -> tuple[bool, Optional[Config]]: + """Handle agent selection response.""" + try: + choice = int(response) + if 1 <= choice <= len(pending.agent_options): + pending.selected_agent_id = pending.agent_options[choice - 1] + await self._show_model_options(pending, send_message) + return True, None + except ValueError: + pass + + await send_message( + pending.channel_id, + pending.thread_ts, + f"Please reply with the number corresponding to the agent.", + ) + return True, None + + async def _handle_model_response( + self, + pending: PendingProjectCreation, + response: str, + send_message: SendMessage, + ) -> tuple[bool, Optional[Config]]: + """Handle model selection response.""" + try: + choice = int(response) + if 1 <= choice <= len(pending.model_options): + selected_model = pending.model_options[choice - 1] + del self._pending_projects[pending.channel_id] + new_config = await self._create_project( + pending, pending.selected_agent_id, selected_model, send_message + ) + return True, new_config + except ValueError: + pass + + await send_message( + pending.channel_id, + pending.thread_ts, + f"Please reply with the number corresponding to the model.", + ) + return True, None + + async def _show_agent_options( + self, + pending: PendingProjectCreation, + send_message: SendMessage, + ) -> None: + """Show available agents for selection.""" + pending.agent_options = list(self._config.agents.keys()) + pending.state = STATE_AWAITING_AGENT + + lines = ["Which agent should I use for this project?\n"] + for i, agent_id in enumerate(pending.agent_options, 1): + lines.append(f"{i}. {agent_id}") + lines.append("\nReply with the number.") + + await send_message(pending.channel_id, pending.thread_ts, "\n".join(lines)) + + async def _show_model_options( + self, + pending: PendingProjectCreation, + send_message: SendMessage, + ) -> None: + """Show available models for the selected agent.""" + agent = self._config.agents.get(pending.selected_agent_id) + if agent and agent.models: + pending.model_options = agent.models.get("available", []) + else: + pending.model_options = [] + + if not pending.model_options: + del self._pending_projects[pending.channel_id] + new_config = await self._create_project( + pending, pending.selected_agent_id, None, send_message + ) + return + + pending.state = STATE_AWAITING_MODEL + + lines = [f"Which model for `{pending.selected_agent_id}`?\n"] + for i, model in enumerate(pending.model_options, 1): + lines.append(f"{i}. {model}") + lines.append("\nReply with the number.") + + await send_message(pending.channel_id, pending.thread_ts, "\n".join(lines)) + + async def _create_project( self, pending: PendingProjectCreation, + agent_id: str, + model: Optional[str], send_message: SendMessage, ) -> Optional[Config]: """ - Handle user approving project creation. + Create the project with selected agent and model. Returns: New Config if successful, None on failure """ + model_str = f"/{model}" if model else "" await send_message( pending.channel_id, pending.thread_ts, - f"Creating project **{pending.channel_name}**...", + f"Creating project `{pending.channel_name}` with `{agent_id} {model_str}`...", ) try: request = ProjectCreationRequest( project_id=pending.channel_name, channel_name=pending.channel_name, - default_agent_id="claude", + default_agent_id=agent_id, + default_model=model, ) await self._project_creator.create_project(request) - # Reload config new_config = load_config(self._config_root) await send_message( pending.channel_id, pending.thread_ts, - f"Okay great, I've set up **{pending.channel_name}**! " + f"Okay great, I've set up `{pending.channel_name}`! " "What's this new idea? What do you want me to do?", ) diff --git a/src/core/config.py b/src/core/config.py index 8a9ba2a..d0a8173 100644 --- a/src/core/config.py +++ b/src/core/config.py @@ -153,6 +153,8 @@ def _load_projects(path: Path) -> Tuple[Dict[str, Project], Path]: if not default_agent: raise ConfigError(f"Project {project_id} missing default_agent") + default_model = cfg.get("default_model") + github_cfg = cfg.get("github") github = None if github_cfg: @@ -170,6 +172,7 @@ def _load_projects(path: Path) -> Tuple[Dict[str, Project], Path]: channel_name=project_id, path=full_path, default_agent_id=default_agent, + default_model=default_model, github=github, ) if not projects: diff --git a/src/core/models.py b/src/core/models.py index 08fbadc..0b9d8d5 100644 --- a/src/core/models.py +++ b/src/core/models.py @@ -35,6 +35,7 @@ class Project: path: Path default_agent_id: str github: Optional[GitHubRepoConfig] = None + default_model: Optional[str] = None class SessionStatus(str, Enum): diff --git a/src/core/project_creation.py b/src/core/project_creation.py index c05ce9d..65f7701 100644 --- a/src/core/project_creation.py +++ b/src/core/project_creation.py @@ -8,6 +8,7 @@ import shutil from dataclasses import dataclass from pathlib import Path +from typing import Optional import yaml @@ -35,6 +36,7 @@ class ProjectCreationRequest: project_id: str channel_name: str default_agent_id: str = "claude" + default_model: Optional[str] = None default_base_branch: str = "main" @@ -137,6 +139,7 @@ async def _handle_existing_directory( repo_name=repo_name, default_base_branch=request.default_base_branch, default_agent_id=request.default_agent_id, + default_model=request.default_model, ) else: raise LocalDirNotGitRepoError( @@ -186,6 +189,7 @@ async def _create_new_project( repo_name=repo_name, default_base_branch=request.default_base_branch, default_agent_id=request.default_agent_id, + default_model=request.default_model, ) LOGGER.info("Updated projects.yaml") @@ -308,6 +312,7 @@ async def _add_to_config( repo_name: str, default_base_branch: str, default_agent_id: str, + default_model: Optional[str] = None, ) -> Project: """Add project entry to projects.yaml.""" projects_yaml = self._config.config_dir / "projects.yaml" @@ -323,7 +328,7 @@ async def _add_to_config( except ValueError: relative_path = local_path - data["projects"][project_id] = { + project_data = { "path": str(relative_path), "default_agent": default_agent_id, "github": { @@ -332,6 +337,10 @@ async def _add_to_config( "default_base_branch": default_base_branch, }, } + if default_model: + project_data["default_model"] = default_model + + data["projects"][project_id] = project_data with open(projects_yaml, "w") as f: yaml.dump(data, f, default_flow_style=False, sort_keys=False) @@ -341,6 +350,7 @@ async def _add_to_config( channel_name=channel_name, path=local_path, default_agent_id=default_agent_id, + default_model=default_model, github=GitHubRepoConfig( owner=github_owner, repo=repo_name, diff --git a/tests/test_router_reactions.py b/tests/test_router_reactions.py index dba87ad..c75278e 100644 --- a/tests/test_router_reactions.py +++ b/tests/test_router_reactions.py @@ -128,6 +128,10 @@ def test_pending_project_creation_fields(self): assert pending.channel_name == "my-project" assert pending.thread_ts == "111.000" assert pending.created_at == 1234567890.0 + assert pending.state == "awaiting_confirmation" + assert pending.selected_agent_id is None + assert pending.agent_options == [] + assert pending.model_options == [] class TestRouterMissingProject: @@ -150,7 +154,7 @@ async def test_missing_project_sends_prompt(self, router_setup): # Should have sent the prompt message assert len(adapter.messages) == 1 assert "new-idea" in adapter.messages[0]["text"] - assert "Reply with **Y**" in adapter.messages[0]["text"] + assert "Reply with \"Y\"" in adapter.messages[0]["text"] @pytest.mark.asyncio async def test_missing_project_tracks_pending(self, router_setup): @@ -178,11 +182,11 @@ class TestRouterYesNoHandling: """Tests for Y/N response handling in Router.""" @pytest.mark.asyncio - async def test_yes_response_creates_project(self, router_setup): - """Test that 'Y' response triggers project creation.""" + async def test_full_creation_flow(self, router_setup): + """Test full flow: Y → pick agent → pick model → project created.""" router, adapter, tmp_path = router_setup - # First, send a message to trigger the prompt + # Step 1: Send message to trigger prompt event1 = { "channel": "C123", "channel_name": "new-idea", @@ -191,7 +195,28 @@ async def test_yes_response_creates_project(self, router_setup): } await router.handle_message(event1) - # Now send 'Y' response + # Step 2: Send 'Y' - should show agent options + event2 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "Y", + "ts": "111.333", + } + await router.handle_message(event2) + assert any("Which agent" in msg["text"] for msg in adapter.messages) + assert any("1. claude" in msg["text"] for msg in adapter.messages) + + # Step 3: Pick agent - should show model options + event3 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "1", + "ts": "111.444", + } + await router.handle_message(event3) + assert any("Which model" in msg["text"] for msg in adapter.messages) + + # Step 4: Pick model - should create project with patch.object( router._project_creation_handler._project_creator, "create_project", new_callable=AsyncMock ) as mock_create: @@ -207,30 +232,25 @@ async def test_yes_response_creates_project(self, router_setup): ), ) - event2 = { + event4 = { "channel": "C123", "channel_name": "new-idea", - "text": "Y", - "ts": "111.333", + "text": "1", + "ts": "111.555", } - await router.handle_message(event2) + await router.handle_message(event4) - # Should have called create_project mock_create.assert_called_once() - # Should have sent "Creating..." and success messages assert any("Creating project" in msg["text"] for msg in adapter.messages) assert any("I've set up" in msg["text"] for msg in adapter.messages) - - # Pending should be cleaned up assert "C123" not in router._project_creation_handler._pending_projects @pytest.mark.asyncio - async def test_yes_lowercase_works(self, router_setup): - """Test that lowercase 'y' also works.""" - router, adapter, tmp_path = router_setup + async def test_yes_shows_agent_options(self, router_setup): + """Test that 'Y' response shows agent options.""" + router, adapter, _ = router_setup - # Trigger the prompt event1 = { "channel": "C123", "channel_name": "new-idea", @@ -239,32 +259,24 @@ async def test_yes_lowercase_works(self, router_setup): } await router.handle_message(event1) - with patch.object( - router._project_creation_handler._project_creator, "create_project", new_callable=AsyncMock - ) as mock_create: - mock_create.return_value = Project( - id="new-idea", - channel_name="new-idea", - path=tmp_path / "projects" / "new-idea", - default_agent_id="claude", - ) - - event2 = { - "channel": "C123", - "channel_name": "new-idea", - "text": "y", - "ts": "111.333", - } - await router.handle_message(event2) + event2 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "y", + "ts": "111.333", + } + await router.handle_message(event2) - mock_create.assert_called_once() + assert any("Which agent" in msg["text"] for msg in adapter.messages) + handler = router._project_creation_handler + assert "C123" in handler._pending_projects + assert handler._pending_projects["C123"].state == "awaiting_agent" @pytest.mark.asyncio async def test_yes_full_word_works(self, router_setup): """Test that 'yes' also works.""" - router, adapter, tmp_path = router_setup + router, adapter, _ = router_setup - # Trigger the prompt event1 = { "channel": "C123", "channel_name": "new-idea", @@ -273,25 +285,15 @@ async def test_yes_full_word_works(self, router_setup): } await router.handle_message(event1) - with patch.object( - router._project_creation_handler._project_creator, "create_project", new_callable=AsyncMock - ) as mock_create: - mock_create.return_value = Project( - id="new-idea", - channel_name="new-idea", - path=tmp_path / "projects" / "new-idea", - default_agent_id="claude", - ) - - event2 = { - "channel": "C123", - "channel_name": "new-idea", - "text": "yes", - "ts": "111.333", - } - await router.handle_message(event2) + event2 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "yes", + "ts": "111.333", + } + await router.handle_message(event2) - mock_create.assert_called_once() + assert any("Which agent" in msg["text"] for msg in adapter.messages) @pytest.mark.asyncio async def test_no_response_rejects(self, router_setup): @@ -370,7 +372,7 @@ async def test_invalid_response_reminds_user(self, router_setup): await router.handle_message(event2) # Should have sent reminder message - assert any("Please reply with **Y**" in msg["text"] for msg in adapter.messages) + assert any("Please reply with \"Y\"" in msg["text"] for msg in adapter.messages) # Pending should still be tracked assert "C123" in router._project_creation_handler._pending_projects @@ -380,7 +382,7 @@ async def test_creation_failure_sends_error(self, router_setup): """Test that failed project creation sends error message.""" router, adapter, _ = router_setup - # Trigger the prompt + # Step 1: Trigger the prompt event1 = { "channel": "C123", "channel_name": "new-idea", @@ -389,19 +391,37 @@ async def test_creation_failure_sends_error(self, router_setup): } await router.handle_message(event1) - # Mock creation to fail + # Step 2: Say Y - shows agent options + event2 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "Y", + "ts": "111.333", + } + await router.handle_message(event2) + + # Step 3: Pick agent - shows model options + event3 = { + "channel": "C123", + "channel_name": "new-idea", + "text": "1", + "ts": "111.444", + } + await router.handle_message(event3) + + # Step 4: Pick model - creation fails with patch.object( router._project_creation_handler._project_creator, "create_project", new_callable=AsyncMock ) as mock_create: mock_create.side_effect = Exception("GitHub API error") - event2 = { + event4 = { "channel": "C123", "channel_name": "new-idea", - "text": "Y", - "ts": "111.333", + "text": "1", + "ts": "111.555", } - await router.handle_message(event2) + await router.handle_message(event4) # Should have sent error message assert any("couldn't create the project" in msg["text"] for msg in adapter.messages) From 7392eaab3e0cc31b2c305cb62936aadae5b9f1fe Mon Sep 17 00:00:00 2001 From: PeterShin23 Date: Thu, 18 Dec 2025 08:12:24 -0500 Subject: [PATCH 4/9] change messages slightly --- src/core/commands/project_creation.py | 12 ++++++------ tests/test_router_reactions.py | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/core/commands/project_creation.py b/src/core/commands/project_creation.py index be6dced..2a6134d 100644 --- a/src/core/commands/project_creation.py +++ b/src/core/commands/project_creation.py @@ -79,8 +79,9 @@ async def handle_missing_project( await send_message( channel_id, thread_ts, - f"I couldn't find a project named `{channel_name}`. Is this a new idea?!?!\n\n" - "Reply with \"Y\" to create it, or \"N\" to cancel.", + f"I couldn't find a project named `{channel_name}`. " + "Is this a new idea, or something that already exists on your computer?\n\n" + "Reply with \"Y\" to set it up, or \"N\" to cancel.", ) # Track pending creation @@ -246,11 +247,11 @@ async def _create_project( Returns: New Config if successful, None on failure """ - model_str = f"/{model}" if model else "" + model_str = f" {model}" if model else "" await send_message( pending.channel_id, pending.thread_ts, - f"Creating project `{pending.channel_name}` with `{agent_id} {model_str}`...", + f"Setting up project `{pending.channel_name}` with `{agent_id}{model_str}`...", ) try: @@ -268,8 +269,7 @@ async def _create_project( await send_message( pending.channel_id, pending.thread_ts, - f"Okay great, I've set up `{pending.channel_name}`! " - "What's this new idea? What do you want me to do?", + f"Okay great, I've set up `{pending.channel_name}`! What do you want me to do?", ) return new_config diff --git a/tests/test_router_reactions.py b/tests/test_router_reactions.py index c75278e..b116e49 100644 --- a/tests/test_router_reactions.py +++ b/tests/test_router_reactions.py @@ -242,7 +242,7 @@ async def test_full_creation_flow(self, router_setup): mock_create.assert_called_once() - assert any("Creating project" in msg["text"] for msg in adapter.messages) + assert any("Setting up project" in msg["text"] for msg in adapter.messages) assert any("I've set up" in msg["text"] for msg in adapter.messages) assert "C123" not in router._project_creation_handler._pending_projects From e852c20dd7531a41c8647d8aed68bb97f044a1d5 Mon Sep 17 00:00:00 2001 From: PeterShin23 Date: Thu, 18 Dec 2025 08:21:50 -0500 Subject: [PATCH 5/9] immediately starting session after new configured project --- src/core/router.py | 18 ++++++++++++++-- tests/test_router_reactions.py | 38 ++++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/core/router.py b/src/core/router.py index be64db1..31fe1ca 100644 --- a/src/core/router.py +++ b/src/core/router.py @@ -149,6 +149,20 @@ async def handle_message(self, event: Dict[str, Any]) -> None: if was_handled: if new_config: self._apply_new_config(new_config) + # Start a session for the newly created project + try: + project = self._config.get_project_by_channel(channel_lookup) + session, _ = self._get_or_create_session(project, channel_id, thread_ts) + # Send the "Starting session" message like we do for existing projects + model_display = f" `{session.active_model}`" if session.active_model else "" + await self._send_message( + channel_id, + thread_ts, + f"Starting session for `{project.id}` with `{session.active_agent_id}`{model_display}. " + "Send a message with your request, or use `!help` for common commands.", + ) + except ProjectNotFound: + pass # Shouldn't happen after successful creation return try: @@ -203,8 +217,8 @@ def _get_or_create_session(self, project: Project, channel_id: str, thread_ts: s return self._session_manager.get_by_thread(channel_id, thread_ts), False except SessionNotFound: default_agent = self._config.get_agent(project.default_agent_id) - # Get default model for the agent - default_model = default_agent.models.get("default") if default_agent.models else None + # Prefer project's default_model (set during creation), fall back to agent's default + default_model = project.default_model or (default_agent.models.get("default") if default_agent.models else None) session = self._session_manager.create_session( project=project, channel_id=channel_id, diff --git a/tests/test_router_reactions.py b/tests/test_router_reactions.py index b116e49..f41ac62 100644 --- a/tests/test_router_reactions.py +++ b/tests/test_router_reactions.py @@ -195,12 +195,13 @@ async def test_full_creation_flow(self, router_setup): } await router.handle_message(event1) - # Step 2: Send 'Y' - should show agent options + # Step 2: Send 'Y' - should show agent options (thread_ts points to original) event2 = { "channel": "C123", "channel_name": "new-idea", "text": "Y", "ts": "111.333", + "thread_ts": "111.222", } await router.handle_message(event2) assert any("Which agent" in msg["text"] for msg in adapter.messages) @@ -212,19 +213,38 @@ async def test_full_creation_flow(self, router_setup): "channel_name": "new-idea", "text": "1", "ts": "111.444", + "thread_ts": "111.222", } await router.handle_message(event3) assert any("Which model" in msg["text"] for msg in adapter.messages) # Step 4: Pick model - should create project + # Write the project to projects.yaml so load_config finds it + config_dir = tmp_path / "config" + projects_yaml = config_dir / "projects.yaml" + project_dir = tmp_path / "projects" / "new-idea" + project_dir.mkdir(parents=True, exist_ok=True) + projects_yaml.write_text(f"""base_dir: "{tmp_path / 'projects'}" +projects: + new-idea: + path: new-idea + default_agent: claude + default_model: sonnet + github: + owner: test-user + repo: new-idea + default_base_branch: main +""") + with patch.object( router._project_creation_handler._project_creator, "create_project", new_callable=AsyncMock ) as mock_create: mock_create.return_value = Project( id="new-idea", channel_name="new-idea", - path=tmp_path / "projects" / "new-idea", + path=project_dir, default_agent_id="claude", + default_model="sonnet", github=GitHubRepoConfig( owner="test-user", repo="new-idea", @@ -237,6 +257,7 @@ async def test_full_creation_flow(self, router_setup): "channel_name": "new-idea", "text": "1", "ts": "111.555", + "thread_ts": "111.222", } await router.handle_message(event4) @@ -246,6 +267,19 @@ async def test_full_creation_flow(self, router_setup): assert any("I've set up" in msg["text"] for msg in adapter.messages) assert "C123" not in router._project_creation_handler._pending_projects + # Session should be automatically created after project setup + session = router._session_manager.get_by_thread("C123", "111.222") + assert session is not None + assert session.active_agent_id == "claude" + assert session.active_model == "sonnet" + + # "Starting session" message should be sent after project setup + assert any("Starting session for" in msg["text"] for msg in adapter.messages) + starting_msg = next(m for m in adapter.messages if "Starting session for" in m["text"]) + assert "new-idea" in starting_msg["text"] + assert "claude" in starting_msg["text"] + assert "sonnet" in starting_msg["text"] + @pytest.mark.asyncio async def test_yes_shows_agent_options(self, router_setup): """Test that 'Y' response shows agent options.""" From a96f91f7be996e3503e4ef555474fa50702f9f56 Mon Sep 17 00:00:00 2001 From: PeterShin23 Date: Thu, 18 Dec 2025 08:29:33 -0500 Subject: [PATCH 6/9] fix git repo add --- src/core/project_creation.py | 36 ++++++++++++++++++++++++----- tests/test_project_creation.py | 41 +++++++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/src/core/project_creation.py b/src/core/project_creation.py index 65f7701..de84009 100644 --- a/src/core/project_creation.py +++ b/src/core/project_creation.py @@ -154,6 +154,7 @@ async def _create_new_project( request: ProjectCreationRequest, ) -> Project: """Create a completely new project with GitHub repo.""" + repo_full_name: Optional[str] = None try: github_owner = await self._get_github_owner() @@ -175,12 +176,16 @@ async def _create_new_project( ) LOGGER.info("Created initial commit") - remote_url = f"git@github.com:{repo_full_name}.git" + # Use HTTPS with token auth instead of SSH for reliability + remote_url = self._get_authenticated_remote_url(repo_full_name) await self._push_to_github( local_path, remote_url, request.default_base_branch ) LOGGER.info("Pushed to GitHub") + # After successful push, update remote to use SSH for future operations + await self._run_git(local_path, ["remote", "set-url", "origin", f"git@github.com:{repo_full_name}.git"]) + project = await self._add_to_config( project_id=repo_name, channel_name=request.channel_name, @@ -196,15 +201,23 @@ async def _create_new_project( return project except (ProjectCreationError, RepoExistsError, LocalDirNotGitRepoError, GitHubError): - self._cleanup_failed_creation(local_path) + self._cleanup_failed_creation(local_path, repo_full_name) raise except Exception as e: - self._cleanup_failed_creation(local_path) + self._cleanup_failed_creation(local_path, repo_full_name) raise ProjectCreationError( f"Something unexpected happened while creating '{repo_name}'. " f"You'll need to check this when you're home. Sorry :( ({e})" ) from e + def _get_authenticated_remote_url(self, repo_full_name: str) -> str: + """Get HTTPS remote URL with token authentication.""" + token = self._config.github_token + if token: + return f"https://x-access-token:{token}@github.com/{repo_full_name}.git" + # Fall back to SSH if no token (shouldn't happen since we check earlier) + return f"git@github.com:{repo_full_name}.git" + def _sanitize_repo_name(self, channel_name: str) -> str: """ Sanitize channel name to a valid GitHub repo name. @@ -375,11 +388,24 @@ async def _run_git(self, cwd: Path, args: list[str]) -> str: return stdout.decode().strip() if stdout else "" - def _cleanup_failed_creation(self, path: Path) -> None: - """Clean up local directory on failure.""" + def _cleanup_failed_creation(self, path: Path, repo_full_name: Optional[str] = None) -> None: + """Clean up local directory and GitHub repo on failure.""" if path.exists(): try: shutil.rmtree(path) LOGGER.info("Cleaned up failed creation: %s", path) except Exception as e: LOGGER.warning("Failed to cleanup %s: %s", path, e) + + if repo_full_name: + try: + self._delete_github_repo(repo_full_name) + LOGGER.info("Deleted GitHub repository: %s", repo_full_name) + except Exception as e: + LOGGER.warning("Failed to delete GitHub repo %s: %s", repo_full_name, e) + + def _delete_github_repo(self, repo_full_name: str) -> None: + """Delete a GitHub repository.""" + owner, repo = repo_full_name.split("/") + gh_repo = self._github._client.get_repo(repo_full_name) + gh_repo.delete() diff --git a/tests/test_project_creation.py b/tests/test_project_creation.py index 5ed0cad..9c15cdc 100644 --- a/tests/test_project_creation.py +++ b/tests/test_project_creation.py @@ -323,9 +323,18 @@ async def test_create_project_sanitizes_channel_name( async def test_create_project_cleanup_on_failure( self, test_config, mock_github_manager ): - """Test that failed creation cleans up local directory.""" + """Test that failed creation cleans up local directory and GitHub repo.""" service = ProjectCreationService(test_config, mock_github_manager) + # Update mock to return correct repo name for this test + mock_created_repo = MagicMock() + mock_created_repo.full_name = "test-user/failed-project" + mock_github_manager._client.get_user().create_repo.return_value = mock_created_repo + + # Set up mock for repo deletion + mock_repo_to_delete = MagicMock() + mock_github_manager._client.get_repo.return_value = mock_repo_to_delete + # Mock git init to succeed but push to fail call_count = 0 @@ -348,6 +357,10 @@ async def mock_git(cwd, args): # Verify local directory was cleaned up assert not (test_config.base_dir / "failed-project").exists() + # Verify GitHub repo deletion was attempted + mock_github_manager._client.get_repo.assert_called_with("test-user/failed-project") + mock_repo_to_delete.delete.assert_called_once() + def test_update_config(self, test_config, mock_github_manager): """Test that update_config updates the internal config reference.""" service = ProjectCreationService(test_config, mock_github_manager) @@ -356,3 +369,29 @@ def test_update_config(self, test_config, mock_github_manager): service.update_config(new_config) assert service._config is new_config + + def test_get_authenticated_remote_url_with_token(self, test_config, mock_github_manager): + """Test that authenticated URL uses HTTPS with token.""" + service = ProjectCreationService(test_config, mock_github_manager) + + url = service._get_authenticated_remote_url("owner/repo") + + assert url == "https://x-access-token:test-token@github.com/owner/repo.git" + + def test_get_authenticated_remote_url_without_token(self, mock_github_manager): + """Test that without token, falls back to SSH.""" + config = Config( + projects={}, + agents={}, + slack_bot_token="bot", + slack_app_token="app", + slack_allowed_user_ids=["U123"], + base_dir=Path("/tmp"), + config_dir=Path("/tmp"), + github_token=None, # No token + ) + service = ProjectCreationService(config, mock_github_manager) + + url = service._get_authenticated_remote_url("owner/repo") + + assert url == "git@github.com:owner/repo.git" From fd896fee636303a7da5494d70f991a4e253e1d80 Mon Sep 17 00:00:00 2001 From: PeterShin23 Date: Fri, 19 Dec 2025 16:58:22 -0500 Subject: [PATCH 7/9] fixing push to remote --- src/core/git_workflow.py | 29 +++++++++++++++++++++++------ src/core/project_creation.py | 3 --- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/core/git_workflow.py b/src/core/git_workflow.py index b7d0316..f991d74 100644 --- a/src/core/git_workflow.py +++ b/src/core/git_workflow.py @@ -20,6 +20,15 @@ LOGGER = logging.getLogger(__name__) +def _get_authenticated_url(project: Project, token: Optional[str]) -> Optional[str]: + """Get HTTPS URL with token authentication for a project.""" + if not project.github or not token: + return None + owner = project.github.owner + repo = project.github.repo + return f"https://x-access-token:{token}@github.com/{owner}/{repo}.git" + + class GitWorkflowService: """Encapsulates git operations and PR publishing logic.""" @@ -81,7 +90,7 @@ async def setup_session_branch(self, session: Session, project: Project) -> None return base = project.github.default_base_branch - await self._prepare_base_branch(repo_path, base, require_clean=True) + await self._prepare_base_branch(repo_path, base, require_clean=True, project=project) await self._run_git(repo_path, ["checkout", "-B", branch, base]) async def stash_changes(self, repo_path: Path) -> bool: @@ -106,7 +115,9 @@ async def _publish_branch_update(self, session: Session, project: Project, pr_ti await self._run_git(session.project_path, ["add", "-A"]) if not await self._commit_changes(session.project_path, pr_title): return None - await self._run_git(session.project_path, ["push", "-u", "origin", branch]) + + remote = _get_authenticated_url(project, self._github_manager.token) or "origin" + await self._run_git(session.project_path, ["push", "-u", remote, branch]) existing_pr_number = self._get_existing_pr_number(session.id) @@ -137,18 +148,24 @@ async def _publish_branch_update(self, session: Session, project: Project, pr_ti self._session_manager.set_pr_ref(pr_ref) return f"Pushed updates to branch `{branch}`\nLinked PR: {pr_ref.url}" - async def _prepare_base_branch(self, repo_path: Path, base: str, require_clean: bool = False) -> None: + async def _prepare_base_branch( + self, repo_path: Path, base: str, require_clean: bool = False, project: Optional[Project] = None + ) -> None: if require_clean and await self._repo_has_changes(repo_path): raise GitHubError( "Working tree has local changes. Run `!stash` to stash them and start a new session." ) - await self._run_git(repo_path, ["fetch", "origin", base]) + # Use authenticated HTTPS URL for fetch/pull to avoid SSH key issues + remote = "origin" + if project: + remote = _get_authenticated_url(project, self._github_manager.token) or "origin" + await self._run_git(repo_path, ["fetch", remote, base]) show_ref = await self._run_git(repo_path, ["show-ref", "--verify", f"refs/heads/{base}"], check=False) if show_ref.returncode != 0: await self._run_git(repo_path, ["checkout", "-B", base, f"origin/{base}"]) else: await self._run_git(repo_path, ["checkout", base]) - await self._run_git(repo_path, ["pull", "--ff-only", "origin", base]) + await self._run_git(repo_path, ["pull", "--ff-only", remote, base]) async def _commit_changes(self, repo_path: Path, message: str) -> bool: try: @@ -172,7 +189,7 @@ async def _ensure_branch(self, repo_path: Path, project: Project, branch: str) - await self._run_git(repo_path, ["checkout", "-b", branch]) return - await self._prepare_base_branch(repo_path, base) + await self._prepare_base_branch(repo_path, base, project=project) await self._run_git(repo_path, ["checkout", "-B", branch, base]) def _get_existing_pr_number(self, session_id: UUID) -> Optional[int]: diff --git a/src/core/project_creation.py b/src/core/project_creation.py index de84009..ca6eb60 100644 --- a/src/core/project_creation.py +++ b/src/core/project_creation.py @@ -183,9 +183,6 @@ async def _create_new_project( ) LOGGER.info("Pushed to GitHub") - # After successful push, update remote to use SSH for future operations - await self._run_git(local_path, ["remote", "set-url", "origin", f"git@github.com:{repo_full_name}.git"]) - project = await self._add_to_config( project_id=repo_name, channel_name=request.channel_name, From ab062b8a5ad2cb2e94d27a8814723a9a30ae7cf1 Mon Sep 17 00:00:00 2001 From: PeterShin23 Date: Fri, 19 Dec 2025 17:04:23 -0500 Subject: [PATCH 8/9] fixing removed token, overall try catch message --- src/core/router.py | 10 +++++++++- src/github/client.py | 5 +++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/core/router.py b/src/core/router.py index 31fe1ca..56b9d43 100644 --- a/src/core/router.py +++ b/src/core/router.py @@ -263,7 +263,15 @@ async def _run_agent_interaction( ) return - await self._agent_runner.run(session, project, channel_id, thread_ts, user_text) + try: + await self._agent_runner.run(session, project, channel_id, thread_ts, user_text) + except Exception as exc: + LOGGER.exception("Unexpected error during agent interaction for session %s", session.id) + await self._send_message( + channel_id, + thread_ts, + f"Something went wrong: {exc}", + ) async def _handle_command( self, diff --git a/src/github/client.py b/src/github/client.py index 32adb4e..ac6690d 100644 --- a/src/github/client.py +++ b/src/github/client.py @@ -48,6 +48,11 @@ def update_token(self, token: Optional[str]) -> None: def is_configured(self) -> bool: return self._client is not None + @property + def token(self) -> Optional[str]: + """Get the GitHub token for git operations.""" + return self._token + async def ensure_pull_request( self, project: Project, From 6c7fe1e5924fb65f4365346e30904ff2c7fbd1db Mon Sep 17 00:00:00 2001 From: PeterShin23 Date: Fri, 19 Dec 2025 17:33:34 -0500 Subject: [PATCH 9/9] updating claude to bypass perms --- config/agents.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/agents.yaml b/config/agents.yaml index 3ceaa56..da22adb 100644 --- a/config/agents.yaml +++ b/config/agents.yaml @@ -12,7 +12,7 @@ agents: - claude - --print - --permission-mode - - acceptEdits + - bypassPermissions - --output-format - stream-json - --verbose