From e360e5b5151af74c6f763cdc518deb9eca393be0 Mon Sep 17 00:00:00 2001 From: Sandro Date: Tue, 27 Jan 2026 16:33:11 +0000 Subject: [PATCH 1/4] Renamed quick actions to slash commands and merged behavior with skills. --- AGENTS.md | 2 +- CHANGELOG.md | 23 +- Provide a code review for the given merg.md | 84 +++++ README.md | 2 +- daiv/automation/agent/constants.py | 15 +- daiv/automation/agent/graph.py | 14 +- daiv/automation/agent/middlewares/__init__.py | 24 -- daiv/automation/agent/middlewares/skills.py | 253 +++++++++++--- daiv/automation/agent/pr_describer/graph.py | 4 +- .../SKILL.md | 4 +- .../agent/skills/code-review/SKILL.md | 32 ++ .../agent/skills/security-audit/SKILL.md | 34 ++ .../agent/skills/skill-creator/SKILL.md | 2 +- daiv/automation/agent/utils.py | 17 + daiv/chat/api/utils.py | 3 +- daiv/chat/api/views.py | 3 +- daiv/codebase/base.py | 15 + daiv/codebase/clients/github/api/callbacks.py | 83 +---- daiv/codebase/clients/gitlab/api/callbacks.py | 85 +---- daiv/codebase/context.py | 12 +- daiv/codebase/repo_config.py | 10 +- daiv/codebase/tasks.py | 6 +- daiv/daiv/settings/components/common.py | 2 +- daiv/quick_actions/actions/__init__.py | 4 - daiv/quick_actions/actions/clone_to_topic.py | 72 ---- daiv/quick_actions/actions/help.py | 60 ---- daiv/quick_actions/base.py | 138 -------- daiv/quick_actions/decorator.py | 31 -- daiv/quick_actions/parser.py | 53 --- daiv/quick_actions/registry.py | 63 ---- daiv/quick_actions/tasks.py | 118 ------- .../templates/quick_actions/invalid_args.txt | 10 - daiv/slash_commands/__init__.py | 1 + daiv/slash_commands/actions/__init__.py | 4 + daiv/slash_commands/actions/clone_to_topic.py | 124 +++++++ daiv/slash_commands/actions/help.py | 48 +++ .../{quick_actions => slash_commands}/apps.py | 8 +- daiv/slash_commands/base.py | 106 ++++++ daiv/slash_commands/decorator.py | 36 ++ daiv/slash_commands/parser.py | 80 +++++ daiv/slash_commands/registry.py | 67 ++++ .../slash_commands}/clone_to_topic_result.txt | 0 .../slash_commands}/error_message.txt | 6 +- .../templates/slash_commands/invalid_args.txt | 10 + .../slash_commands/slash_commands_help.txt} | 2 +- docker/production/app/Dockerfile | 2 +- docs/configuration/yaml-config.md | 6 +- .../{quick-actions.md => slash-commands.md} | 93 +++--- docs/index.md | 6 +- evals/conftest.py | 3 +- evals/swebench.py | 3 +- evals/test_pr_describer.py | 6 +- mkdocs.yml | 2 +- pyproject.toml | 2 +- tests/unit_tests/codebase/test_config.py | 14 +- .../unit_tests/quick_actions/test_actions.py | 117 ------- tests/unit_tests/quick_actions/test_base.py | 51 --- tests/unit_tests/quick_actions/test_parser.py | 163 --------- .../unit_tests/quick_actions/test_registry.py | 219 ------------- tests/unit_tests/quick_actions/test_tasks.py | 265 --------------- .../unit_tests/slash_commands}/__init__.py | 0 .../slash_commands/actions/__init__.py | 0 .../slash_commands/actions/test_help.py | 32 ++ .../test_decorator.py | 53 +-- .../unit_tests/slash_commands/test_parser.py | 309 ++++++++++++++++++ .../slash_commands/test_registry.py | 220 +++++++++++++ 66 files changed, 1587 insertions(+), 1749 deletions(-) create mode 100644 Provide a code review for the given merg.md rename daiv/automation/agent/skills/{creating-agents-md-file => agentsmd-creation}/SKILL.md (97%) create mode 100644 daiv/automation/agent/skills/code-review/SKILL.md create mode 100644 daiv/automation/agent/skills/security-audit/SKILL.md delete mode 100644 daiv/quick_actions/actions/__init__.py delete mode 100644 daiv/quick_actions/actions/clone_to_topic.py delete mode 100644 daiv/quick_actions/actions/help.py delete mode 100644 daiv/quick_actions/base.py delete mode 100644 daiv/quick_actions/decorator.py delete mode 100644 daiv/quick_actions/parser.py delete mode 100644 daiv/quick_actions/registry.py delete mode 100644 daiv/quick_actions/tasks.py delete mode 100644 daiv/quick_actions/templates/quick_actions/invalid_args.txt create mode 100644 daiv/slash_commands/__init__.py create mode 100644 daiv/slash_commands/actions/__init__.py create mode 100644 daiv/slash_commands/actions/clone_to_topic.py create mode 100644 daiv/slash_commands/actions/help.py rename daiv/{quick_actions => slash_commands}/apps.py (61%) create mode 100644 daiv/slash_commands/base.py create mode 100644 daiv/slash_commands/decorator.py create mode 100644 daiv/slash_commands/parser.py create mode 100644 daiv/slash_commands/registry.py rename daiv/{quick_actions/templates/quick_actions => slash_commands/templates/slash_commands}/clone_to_topic_result.txt (100%) rename daiv/{quick_actions/templates/quick_actions => slash_commands/templates/slash_commands}/error_message.txt (66%) create mode 100644 daiv/slash_commands/templates/slash_commands/invalid_args.txt rename daiv/{quick_actions/templates/quick_actions/quick_actions_help.txt => slash_commands/templates/slash_commands/slash_commands_help.txt} (77%) rename docs/features/{quick-actions.md => slash-commands.md} (61%) delete mode 100644 tests/unit_tests/quick_actions/test_actions.py delete mode 100644 tests/unit_tests/quick_actions/test_base.py delete mode 100644 tests/unit_tests/quick_actions/test_parser.py delete mode 100644 tests/unit_tests/quick_actions/test_registry.py delete mode 100644 tests/unit_tests/quick_actions/test_tasks.py rename {daiv/quick_actions => tests/unit_tests/slash_commands}/__init__.py (100%) create mode 100644 tests/unit_tests/slash_commands/actions/__init__.py create mode 100644 tests/unit_tests/slash_commands/actions/test_help.py rename tests/unit_tests/{quick_actions => slash_commands}/test_decorator.py (50%) create mode 100644 tests/unit_tests/slash_commands/test_parser.py create mode 100644 tests/unit_tests/slash_commands/test_registry.py diff --git a/AGENTS.md b/AGENTS.md index 632d6766..001c6f10 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,7 +9,7 @@ DAIV is an AI-powered development assistant built on Django with Django Tasks fo * `codebase/` - Codebase module with all the repository interaction and related logic. * `chat/` - Chat module with the OpenAI compatible API. * `core/` - Core module with common logic. - * `quick_actions/` - Quick actions module. + * `slash_commands/` - Slash commands module. * `daiv/` - Main logic of the Django project: settings, urls, wsgi, asgi, tasks, etc. * `docker/` - Dockerfiles and configurations for local and production deployments. * `docs/` - Documentation for the project. diff --git a/CHANGELOG.md b/CHANGELOG.md index 795d1823..9617b326 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Added changelog subagent for maintaining changelogs and release notes across any format (CHANGELOG.md, CHANGES.rst, HISTORY.md, NEWS, etc.) with automatic format detection and convention preservation +- Added code review and security audit agent skills for structured review and security guidance. - Added support for issue labels to configure plan and execute agent behavior: - `daiv-auto`: Automatically approve the plan and proceed with implementation without manual approval - `daiv-max`: Use high-performance mode with `CLAUDE_OPUS_4_5` model and `HIGH` thinking level for both planning and execution @@ -20,7 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Improved documentation for Review Addressor with clear examples showing how to address code review comments using direct mentions (`@daiv `). -- Added comparison table to Quick Actions documentation clarifying the difference between slash commands and direct mentions. +- Added comparison table to Slash Commands documentation clarifying the difference between slash commands and direct mentions. - Added configuration section to Issue Addressor documentation with `.daiv.yml` snippets for enabling automated issue resolution and plan approval workflow. - Updated the `generating-agents-md` skill prompt to align with the AGENTS.md creation guidance format. - Updated issue addressing to accept any DAIV label (`daiv`, `daiv-auto`, `daiv-max`) as a trigger. **BREAKING CHANGE**: Issue title prefix (`DAIV:`) is no longer supported as a trigger. Use labels instead. @@ -43,9 +44,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed +- Removed `/review` and `/security-audit` slash commands in favor of builtin skills. - Removed builtin `maintaining-changelog` skill in favor of the new changelog subagent - Removed `pull_request.branch_name_convention` from `.daiv.yml` configuration file. **BREAKING CHANGE**: Branch name convention must now be defined in the `AGENTS.md` file instead. - Removed Celery worker configuration and bootstrap scripts. +- Removed the `quick_actions` Django app, templates, and tests in favor of the `slash_commands` module. ## [1.1.0] - 2025-12-04 @@ -61,7 +64,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added OpenRouter support to Anthropic caching middleware, reducing costs. - Added `FileNavigationMiddleware`, `FileEditingMiddleware`, `MergeRequestMiddleware` and `WebSearchMiddleware` in replacement of toolkits, leveraging LangChain v1 middlewares capabilities to inject the system prompt and tools into the model call. - Added `EXECUTION_THINKING_LEVEL` configuration to `PlanAndExecuteAgent` to allow users to enable thinking for execution tasks. -- Added `/clone-to-topic` quick action to clone issues to all repositories matching specified topics, enabling bulk distribution of issues across multiple repositories. +- Added `/clone-to-topic` slash command to clone issues to all repositories matching specified topics, enabling bulk distribution of issues across multiple repositories. ### Changed @@ -119,10 +122,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Migrated project from Python 3.13 to Python 3.14. - Refactored repository configuration file schema to be more flexible and easier to use. **BREAKING CHANGE** - Moved tools from `daiv/automation/tools` to `daiv/automation/agents/tools`. -- Moved quick actions from `daiv/automation/quick_actions` to `daiv/quick_actions`. -- Migrated quick action `help` to activate as `@daiv /help` instead of `@daiv help`. **BREAKING CHANGE** -- Migrated quick action `plan execute` to activate as `@daiv /approve-plan` instead of `@daiv plan execute`. **BREAKING CHANGE** -- Migrated quick action `plan revise` to activate as `@daiv /revise-plan` instead of `@daiv plan revise`. **BREAKING CHANGE** +- Moved slash commands from `daiv/automation/slash_commands` to `daiv/slash_commands`. +- Migrated slash command `help` to activate as `@daiv /help` instead of `@daiv help`. **BREAKING CHANGE** +- Migrated slash command `plan execute` to activate as `@daiv /approve-plan` instead of `@daiv plan execute`. **BREAKING CHANGE** +- Migrated slash command `plan revise` to activate as `@daiv /revise-plan` instead of `@daiv plan revise`. **BREAKING CHANGE** - Updated project dependencies. - Updated documentation. @@ -154,14 +157,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Added quick actions feature to allow users perform actions by commenting on the merge request or issue. -- Added quick actions to allow users to trigger plan revision by commenting `@daiv plan revise` on the issue. +- Added slash commands feature to allow users perform actions by commenting on the merge request or issue. +- Added slash commands to allow users to trigger plan revision by commenting `@daiv plan revise` on the issue. ### Changed - Migrated `RunSandboxCommandsTool` and `RunSandboxCodeTool` to be async only. -- Migrated `PipelineFixerAgent` to be triggered by a quick action instead of a webhook, allowing users to request a repair plan to fix pipelines by commenting `@daiv pipeline repair` on the merge request. -- Migrated `IssueAddressorAgent` plan approval to be triggered by a quick action, allowing users to request a plan approval by commenting `@daiv plan execute` on the issue. +- Migrated `PipelineFixerAgent` to be triggered by a slash command instead of a webhook, allowing users to request a repair plan to fix pipelines by commenting `@daiv pipeline repair` on the merge request. +- Migrated `IssueAddressorAgent` plan approval to be triggered by a slash command, allowing users to request a plan approval by commenting `@daiv plan execute` on the issue. ### Fixed diff --git a/Provide a code review for the given merg.md b/Provide a code review for the given merg.md new file mode 100644 index 00000000..7001ee01 --- /dev/null +++ b/Provide a code review for the given merg.md @@ -0,0 +1,84 @@ +Provide a code review for the given merge request. + +**Agent assumptions (applies to all agents and subagents):** +- All tools are functional and will work without error. Do not test tools or make exploratory calls. Make sure this is clear to every subagent that is launched. +- Only call a tool if it is required to complete the task. Every tool call should have a clear purpose. + +To do this, follow these steps precisely: + +1. Launch a general purpose agent to check if any of the following are true: + - The merge request is closed + - The merge request is a draft + - The merge request does not need code review (e.g. automated PR, trivial change that is obviously correct) + + If any condition is true, stop and do not proceed. + +Note: Still review DAIV generated MR's. + +2. Launch a general purpose agent to view the merge request and return a summary of the changes + +3. Launch 3 agents in parallel to independently review the changes. Each agent should return the list of issues, where each issue includes a description and the reason it was flagged (e.g. "AGENTS.md adherence", "bug"). The agents should do the following: + + Agent 1: AGENTS.md compliance agent + Audit changes for AGENTS.md compliance. + + Agent 2: General purpose bug agent (parallel subagent with agent 3) + Scan for obvious bugs. Focus only on the diff itself without reading extra context. Flag only significant bugs; ignore nitpicks and likely false positives. Do not flag issues that you cannot validate without looking at context outside of the git diff. + + Agent 3: General purpose bug agent (parallel subagent with agent 2) + Look for problems that exist in the introduced code. This could be security issues, incorrect logic, etc. Only look for issues that fall within the changed code. + + **CRITICAL: We only want HIGH SIGNAL issues.** Flag issues where: + - The code will fail to compile or parse (syntax errors, type errors, missing imports, unresolved references) + - The code will definitely produce wrong results regardless of inputs (clear logic errors) + - Clear, unambiguous AGENTS.md violations where you can quote the exact rule being broken + + Do NOT flag: + - Code style or quality concerns + - Potential issues that depend on specific inputs or state + - Subjective suggestions or improvements + + If you are not certain an issue is real, do not flag it. False positives erode trust and waste reviewer time. + + In addition to the above, each subagent should be told the PR ID, title and description. This will help provide context regarding the author's intent. + +4. For each issue found in the previous step by agents 2 and 3, launch parallel subagents to validate the issue. These subagents should get the PR ID, title and description along with a description of the issue. The agent's job is to review the issue to validate that the stated issue is truly an issue with high confidence. For example, if an issue such as "variable is not defined" was flagged, the subagent's job would be to validate that is actually true in the code. Another example would be AGENTS.md issues. The agent should validate that the AGENTS.md rule that was violated is scoped for this file and is actually violated. Use subagents for bugs, logic issues, and AGENTS.md violations. + +5. Filter out any issues that were not validated in step 4. This step will give us our list of high signal issues for our review. + +6. If issues were found, skip to step 7 to reply. + + If NO issues were found, reply with the following message: + "No issues found. Checked for bugs and AGENTS.md compliance." + +7. Create a list of all comments that you plan on leaving. This is only for you to make sure you are comfortable with the comments. Do not post this list anywhere. + +8. Post inline comments for each issue using `gitlab` tool with `project-merge-request-draft-note` providing a `position` argument. For each comment: + - Provide a brief description of the issue + - For small, self-contained fixes, include a committable suggestion block + - For larger fixes (6+ lines, structural changes, or changes spanning multiple locations), describe the issue and suggested fix without a suggestion block + - Never post a committable suggestion UNLESS committing the suggestion fixes the issue entirely. If follow up steps are required, do not leave a committable suggestion. + + **IMPORTANT: Only post ONE comment per unique issue. Do not post duplicate comments.** + +Use this list when evaluating issues in Steps 4 and 5 (these are false positives, do NOT flag): + +- Pre-existing issues +- Something that appears to be a bug but is actually correct +- Pedantic nitpicks that a senior engineer would not flag +- Issues that a linter will catch (do not run the linter to verify) +- General code quality concerns (e.g., lack of test coverage, general security issues) unless explicitly required in AGENTS.md +- Issues mentioned in AGENTS.md but explicitly silenced in the code (e.g., via a lint ignore comment) + +Notes: + +- Use `gitlab` tool to interact with GitLab (e.g., fetch merge requests, create inline comments). Do not use web fetch. +- Create a todo list before starting. +- You must cite and link each issue in inline comments (e.g., if referring to a AGENTS.md, include a link to it). +- When linking to code in inline comments, follow the following format precisely, otherwise the Markdown preview won't render correctly: http://gitlab:8929/anthropics/claude-code/-/blob/c21d3c10bc8e898b7ac1a2d745bdc9bc4e423afe/package.json#L10-L15 + - Requires full git sha + - You must provide the full sha. Commands like `http://gitlab:8929/owner/repo/-/blob/$(git rev-parse HEAD)/foo/bar` will not work, since your comment will be directly rendered in Markdown. + - Repo name must match the repo you're code reviewing + - # sign after the file name + - Line range format is L[start]-L[end] + - Provide at least 1 line of context before and after, centered on the line you are commenting about (eg. if you are commenting about lines 5-6, you should link to `L4-7`) diff --git a/README.md b/README.md index d7235c9b..64c8a75a 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ DAIV is an open-source automation assistant designed to enhance developer produc - 💬 **Code Review Addressor**: Assists with code review comments by providing context-aware answers or directly applying requested changes. This reduces the overhead of going back and forth on merge requests. - 🧠 **Codebase Chat**: Chat with your codebase for context-aware answers. An OpenAI-compatible API is available for easy integration with tools such as [Open-WebUI](https://github.com/open-webui/open-webui). - ⚙️ **Configurable Behavior**: A `.daiv.yml` file in your repo's default branch lets you tailor DAIV's features (like toggling auto-issue addressing). -- ⚡ **Quick Actions**: Command-based interactions for common tasks on issues and merge requests, such as regenerating plan, approving plan, repairing pipeline, etc. +- ⚡ **Slash Commands**: Command-based interactions for common tasks on issues and merge requests, such as help to list available commands, cloning issues to multiple repositories, etc. - 🔧 **MCP Tools**: Supporting [Model Context Protocol (MCP)](https://modelcontextprotocol.io/) tools to extend the capabilities of the agents. - 🎯 **Agent Skills**: Modular, reusable capabilities that give agents domain-specific expertise. Define custom workflows in your repository's `.daiv/skills/` directory, or use builtin skills like changelog maintenance and AGENTS.md generation. - 📦 **Sandbox**: Running commands in a secure sandbox to allow the agents to perform actions on the codebase, such as installing/updating dependencies, generating translations, etc. with your own docker image. diff --git a/daiv/automation/agent/constants.py b/daiv/automation/agent/constants.py index 4078b4e7..ab050cfe 100644 --- a/daiv/automation/agent/constants.py +++ b/daiv/automation/agent/constants.py @@ -2,14 +2,19 @@ from daiv.settings.components import PROJECT_DIR -# Path where the builtin skills are stored on the filesystem to be copied to the backend. +# Path where the builtin skills are stored in the filesystem to be copied to the repository. BUILTIN_SKILLS_PATH = PROJECT_DIR / "automation" / "agent" / "skills" -# Path where the project skills are stored in repository. -PROJECT_SKILLS_PATH = ".daiv/skills" +# Path where the skills are stored in repository. +DAIV_SKILLS_PATH = ".daiv/skills" +CURSOR_SKILLS_PATH = ".cursor/skills" +CLAUDE_CODER_SKILLS_PATH = ".claude/skills" -# Path where the project memory is stored in repository. -PROJECT_MEMORY_PATH = ".daiv/AGENTS.md" +# Paths where the skills are stored in repository. +SKILLS_SOURCES = [DAIV_SKILLS_PATH, CURSOR_SKILLS_PATH, CLAUDE_CODER_SKILLS_PATH] + +# Path where the memory is stored in repository. +DAIV_MEMORY_PATH = ".daiv/AGENTS.md" class ModelName(StrEnum): diff --git a/daiv/automation/agent/graph.py b/daiv/automation/agent/graph.py index 9109b8c0..c3a58b10 100644 --- a/daiv/automation/agent/graph.py +++ b/daiv/automation/agent/graph.py @@ -25,7 +25,7 @@ from automation.agent.base import BaseAgent, ThinkingLevel from automation.agent.conf import settings -from automation.agent.constants import PROJECT_MEMORY_PATH, PROJECT_SKILLS_PATH +from automation.agent.constants import DAIV_MEMORY_PATH, SKILLS_SOURCES from automation.agent.mcp.toolkits import MCPToolkit from automation.agent.middlewares.file_system import FilesystemMiddleware from automation.agent.middlewares.git import GitMiddleware @@ -43,6 +43,7 @@ create_general_purpose_subagent, ) from automation.conf import settings as automation_settings +from codebase.base import Scope from codebase.context import RuntimeCtx, set_runtime_ctx from core.constants import BOT_NAME @@ -184,9 +185,9 @@ async def create_daiv_agent( ), MemoryMiddleware( backend=backend, - sources=[f"/{agent_path.name}/{ctx.config.context_file_name}", f"/{agent_path.name}/{PROJECT_MEMORY_PATH}"], + sources=[f"/{agent_path.name}/{ctx.config.context_file_name}", f"/{agent_path.name}/{DAIV_MEMORY_PATH}"], ), - SkillsMiddleware(backend=backend, sources=[f"/{agent_path.name}/{PROJECT_SKILLS_PATH}"]), + SkillsMiddleware(backend=backend, sources=[f"/{agent_path.name}/{source}" for source in SKILLS_SOURCES]), SubAgentMiddleware( default_model=model, default_middleware=subagent_default_middlewares, @@ -234,12 +235,9 @@ async def main(): wrap_lines=True, reserve_space_for_menu=7, # Reserve space for completion menu to show 5-6 results ) - async with set_runtime_ctx(repo_id="srtab/daiv", ref="main") as ctx: + async with set_runtime_ctx(repo_id="srtab/daiv", scope=Scope.GLOBAL, ref="main") as ctx: agent = await create_daiv_agent( - ctx=ctx, - model_names=["openrouter:minimax/minimax-m2.1"], - store=InMemoryStore(), - checkpointer=InMemorySaver(), + ctx=ctx, model_names=["openrouter:z-ai/glm-4.7"], store=InMemoryStore(), checkpointer=InMemorySaver() ) while True: user_input = await session.prompt_async() diff --git a/daiv/automation/agent/middlewares/__init__.py b/daiv/automation/agent/middlewares/__init__.py index f1147151..e69de29b 100644 --- a/daiv/automation/agent/middlewares/__init__.py +++ b/daiv/automation/agent/middlewares/__init__.py @@ -1,24 +0,0 @@ -import logging - -from langchain_core.tools import tool - -logger = logging.getLogger("daiv.tools") - -# https://www.anthropic.com/engineering/claude-think-tool - -THINK_TOOL_NAME = "think" - - -@tool(THINK_TOOL_NAME, parse_docstring=True) -def think_tool(thought: str): - """ - Use the tool to think about something in private. It will not obtain new information or make any changes, but just log the thought. Use it when complex reasoning or brainstorming is needed. Use it as a private scratchpad. - - Args: - thought (str): Your private thoughts. - - Returns: - A message indicating that the thought has been logged. - """ # noqa: E501 - logger.info("[%s] Thinking about: %s", think_tool.name, thought) - return "Thought registered." diff --git a/daiv/automation/agent/middlewares/skills.py b/daiv/automation/agent/middlewares/skills.py index 799fd30d..93bb5ad9 100644 --- a/daiv/automation/agent/middlewares/skills.py +++ b/daiv/automation/agent/middlewares/skills.py @@ -1,85 +1,162 @@ +import logging +import shlex from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Annotated, override from deepagents.middleware.skills import SkillMetadata, SkillsState, SkillsStateUpdate from deepagents.middleware.skills import SkillsMiddleware as DeepAgentsSkillsMiddleware - -from automation.agent.constants import BUILTIN_SKILLS_PATH, PROJECT_SKILLS_PATH +from langchain.agents.middleware import hook_config +from langchain.tools import ToolRuntime, tool # noqa: TC002 +from langchain_core.messages import AIMessage, AnyMessage, HumanMessage, ToolMessage +from langchain_core.prompts import PromptTemplate +from langgraph.runtime import Runtime # noqa: TC002 + +from automation.agent.constants import BUILTIN_SKILLS_PATH, DAIV_SKILLS_PATH +from automation.agent.utils import extract_body_from_frontmatter, extract_text_content +from codebase.context import RuntimeCtx # noqa: TC001 +from slash_commands.parser import SlashCommandCommand, parse_agent_slash_command +from slash_commands.registry import slash_command_registry if TYPE_CHECKING: + from collections.abc import Callable + + from deepagents.backends import BackendProtocol from langchain_core.runnables import RunnableConfig - from langgraph.runtime import Runtime + from langchain_core.tools import BaseTool - from codebase.context import RuntimeCtx +logger = logging.getLogger("daiv.tools") -SKILLS_SYSTEM_PROMPT = """\ -## Skills System +SKILL_ARGUMENTS_PLACEHOLDER = "$ARGUMENTS" -You have access to a skills library that provides specialized capabilities and domain knowledge. +SKILLS_TOOL_NAME = "skill" +SKILLS_TOOL_DESCRIPTION = """Execute a skill within the main conversation. -{skills_locations} +Usage notes: + - Use this tool with the skill name and optional arguments + - If the skill does not exist, the tool will return an error. + - Only use skills listed in . -**Available Skills:** +Examples: + - `skill: "pdf"` - invoke the pdf skill + - `skill: "code-review", skill_args: ["my-branch"]` - invoke with arguments +""" -{skills_list} +SKILLS_SYSTEM_PROMPT = f"""\ +## Skills -**How to Use Skills (Progressive Disclosure):** +**When to Use Skills:** +- When users ask you to perform tasks, check if any of the available skills below can help complete the task more effectively. Skills provide specialized capabilities and domain knowledge. +- When users ask you to run a "slash command" or reference "/" (e.g., "/security-audit", "/code-review"), they are referring to a skill. Use the `{SKILLS_TOOL_NAME}` tool to invoke the corresponding skill. -Skills follow a **progressive disclosure** pattern - you see their name and description above, but only read full instructions when needed: + + User: "run /code-review" + Assistant: [Calls `{SKILLS_TOOL_NAME}` tool with skill name: "code-review"] + -1. **Recognize when a skill applies**: Check if the user's task matches a skill's description -2. **Read the skill's full instructions**: Use the path shown in the skill list above and read the `SKILL.md` file -3. **Follow the skill's instructions**: SKILL.md contains step-by-step workflows, best practices, and examples -4. **Access supporting files**: Skills may include helper scripts, configs, or reference docs - use absolute paths to access them +**Important:** +- When a skill is relevant, you must invoke the `{SKILLS_TOOL_NAME}` tool IMMEDIATELY as your first action. +- NEVER just announce or mention a skill in your text response without actually calling the `{SKILLS_TOOL_NAME}` tool. +- This is a BLOCKING REQUIREMENT: invoke the relevant `{SKILLS_TOOL_NAME}` tool BEFORE generating any other response about the task. +- Only use skills listed in below. +- Do not invoke a skill that is already running. + +{{skills_list}}""" # noqa: E501 + + +AVAILABLE_SKILLS_TEMPLATE = PromptTemplate.from_template( + """ + {{#skills_list}} + + {{name}} + {{description}} + {{#metadata.is_builtin}} + true + {{/metadata.is_builtin}} + + {{/skills_list}} +""", + template_format="mustache", +) + + +def _skill_tool_generator(backend: BackendProtocol | Callable[[ToolRuntime], BackendProtocol]) -> BaseTool: + """ + Generate a skill tool. -**When to Use Skills:** -- User's request matches a skill's domain (e.g., "research X" -> web-research skill) -- You need specialized knowledge or structured workflows -- A skill provides proven patterns for complex tasks + Args: + backend: The backend to read the skill from. -**Executing Skill Scripts:** -Skills may contain Python scripts or other executable files. -Always use absolute paths from the skill list to execute them and use the bash tool when you need to run scripts. + Returns: + A BaseTool. + """ -**Builtin Skills Are Available in the Project Directory:** -Builtin skills are copied into the project's skills directory at agent startup so you can access their `SKILL.md` and -supporting files through the normal filesystem tools. These copied skill folders include a `.gitignore` to keep them -out of commits by default. + async def skill_tool( + skill: Annotated[str, "The skill name. E.g. 'code-review' or 'web-research'"], + runtime: ToolRuntime[RuntimeCtx, SkillsState], + skill_args: Annotated[str | None, "Optional arguments to pass to the skill."] = None, + ) -> str: + """ + Tool to execute a skill. + """ + available_skills = runtime.state["skills_metadata"] + loaded_skill = next( + (skill_metadata for skill_metadata in available_skills if skill_metadata["name"] == skill), None + ) -**Editing Builtin Skills:** -If a user asks to change a builtin skill and expects the change to be committed, delete the `.gitignore` inside that -builtin skill directory before editing so the files are tracked by git. + if loaded_skill is None: + available_skills_names = [skill_metadata["name"] for skill_metadata in available_skills] + return f"error: Skill '{skill}' not found. Available skills: {', '.join(available_skills_names)}." -**Example Workflow:** - -User: "Can you research the latest developments in quantum computing?" + responses = backend.download_files([loaded_skill["path"]]) + if responses[0].error: + return f"error: Failed to launch skill '{skill}': {responses[0].error}. {responses[0].error_message}" -Assistant: Check available skills -> See "web-research" skill with its path -Assistant: Read the skill using the path shown -Assistant: Follow the skill's research workflow (search -> organize -> synthesize) -Assistant: Use any helper scripts with absolute paths to execute them with the bash tool - + body = extract_body_from_frontmatter(responses[0].content.decode("utf-8").strip()) + + # Positional args like $1, $2 + for i, a in enumerate(shlex.split(skill_args or ""), start=1): + body = body.replace(f"${i}", a).replace(f"{SKILL_ARGUMENTS_PLACEHOLDER}[{i}]", a) + + # Named args, only $ARGUMENTS supported + if arg_str := skill_args.strip(): + body = ( + body.replace(SKILL_ARGUMENTS_PLACEHOLDER, arg_str) + if SKILL_ARGUMENTS_PLACEHOLDER in body + else f"{body}\n\n{SKILL_ARGUMENTS_PLACEHOLDER}: {arg_str}" + ) -Remember: Skills make you more capable and consistent. When in doubt, check if a skill exists for the task!""" # noqa: E501 + return [ + ToolMessage(content=f"Launching skill '{skill}'...", tool_call_id=runtime.tool_call_id), + HumanMessage(content=body), + ] + + return tool(SKILLS_TOOL_NAME, description=SKILLS_TOOL_DESCRIPTION)(skill_tool) class SkillsMiddleware(DeepAgentsSkillsMiddleware): """ - Rewrite the DeepAgentsSkillsMiddleware to copy the builtin skills to the project skills directory to make - them available to the agent even if the project skills directory is not set up. + Middleware to apply builtin slash commands early in the conversation and copy builtin skills to the project skills + directory to make them available to the agent even if the project skills directory is not set up. """ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.system_prompt_template = SKILLS_SYSTEM_PROMPT + self.tools = [_skill_tool_generator(self._backend)] + @hook_config(can_jump_to=["end"]) async def abefore_agent( self, state: SkillsState, runtime: Runtime[RuntimeCtx], config: RunnableConfig ) -> SkillsStateUpdate | None: """ - Copy builtin skills to the project skills directory to make them available to the agent. + Apply builtin slash commands early in the conversation and copy builtin skills to the project skills directory + to make them available to the agent. """ + builtin_slash_commands = await self._apply_builtin_slash_commands(state["messages"], runtime.context) + if builtin_slash_commands: + return builtin_slash_commands + if "skills_metadata" in state: return None @@ -116,7 +193,7 @@ async def _copy_builtin_skills(self, agent_path: Path) -> list[str]: """ builtin_skills = [] files_to_upload = [] - project_skills_path = Path(f"/{agent_path.name}/{PROJECT_SKILLS_PATH}") + project_skills_path = Path(f"/{agent_path.name}/{DAIV_SKILLS_PATH}") for builtin_skill_dir in BUILTIN_SKILLS_PATH.iterdir(): if not builtin_skill_dir.is_dir() or builtin_skill_dir.name == "__pycache__": @@ -139,21 +216,91 @@ async def _copy_builtin_skills(self, agent_path: Path) -> list[str]: raise RuntimeError(f"Failed to upload builtin skill: {response.error}") return builtin_skills + @override def _format_skills_list(self, skills: list[SkillMetadata]) -> str: """ Format the skills list for the system prompt. + + Args: + skills: The list of skills. + + Returns: + The formatted skills list. """ if not skills: paths = [f"{source_path}" for source_path in self.sources] return f"(No skills available yet. You can create skills in {' or '.join(paths)})" - lines = [] - for skill in skills: - metadata = skill.get("metadata", {}) - if metadata.get("is_builtin"): - lines.append(f"- **{skill['name']} (Builtin)**: {skill['description']}") - else: - lines.append(f"- **{skill['name']}**: {skill['description']}") - lines.append(f" -> Read `{skill['path']}` for full instructions") + return AVAILABLE_SKILLS_TEMPLATE.format(skills_list=skills) + + async def _apply_builtin_slash_commands( + self, messages: list[AnyMessage], context: RuntimeCtx + ) -> SkillsStateUpdate | None: + """ + Detect and execute builtin slash commands (not project skills) early in the conversation. + + Args: + messages: The list of messages. + context: The runtime context. + + Returns: + State update with messages injected, or None if no builtin slash command detected. + """ + slash_command = self._extract_slash_command(messages, context.bot_username) + if not slash_command: + return None + + command_classes = slash_command_registry.get_commands(scope=context.scope, command=slash_command.command) + if not command_classes: + return None + + if len(command_classes) > 1: + logger.warning( + "[%s] Multiple `%s` slash commands found for scope '%s': %r", + self.name, + slash_command.command, + context.scope.value, + [c.command for c in command_classes], + ) + return None + + command = command_classes[0]() + logger.info("[%s] Executing `%s` slash command", self.name, slash_command.raw) + + try: + result = await command.execute_for_agent( + args=" ".join(slash_command.args), + scope=context.scope, + repo_id=context.repo_id, + bot_username=context.bot_username, + issue_iid=context.issue.iid if context.issue else None, + merge_request_id=context.merge_request.merge_request_id if context.merge_request else None, + ) + except Exception: + logger.exception("[%s] Failed to execute `%s` slash command", self.name, slash_command.raw) + return {"messages": [AIMessage(content=f"Failed to execute `{slash_command.raw}`.")], "jump_to": "end"} + else: + logger.info("[%s] `%s` slash command completed", self.name, slash_command.raw) + return {"messages": [AIMessage(content=result)], "jump_to": "end"} + + def _extract_slash_command(self, messages: list[AnyMessage], bot_username: str) -> SlashCommandCommand | None: + """ + Extract the slash command from the latest message. + + Args: + messages: The list of messages. + bot_username: The username of the bot. + + Returns: + The slash command command if found, otherwise None. + """ + latest_message = messages[-1] + + if not hasattr(latest_message, "type") or latest_message.type != "human": + return None + + text_content = extract_text_content(latest_message.content) + if not text_content or not text_content.strip(): + return None - return "\n".join(lines) + return parse_agent_slash_command(text_content, bot_username) diff --git a/daiv/automation/agent/pr_describer/graph.py b/daiv/automation/agent/pr_describer/graph.py index aeda13a1..e8922a09 100644 --- a/daiv/automation/agent/pr_describer/graph.py +++ b/daiv/automation/agent/pr_describer/graph.py @@ -12,7 +12,7 @@ from langchain_core.prompts import ChatPromptTemplate from automation.agent import BaseAgent -from automation.agent.constants import PROJECT_MEMORY_PATH +from automation.agent.constants import DAIV_MEMORY_PATH from automation.agent.middlewares.prompt_cache import AnthropicPromptCachingMiddleware from codebase.context import RuntimeCtx @@ -58,7 +58,7 @@ def create_pr_describer_agent(model: ModelName | str, *, ctx: RuntimeCtx) -> Run backend=backend, sources=[ f"/{agent_path.name}/{ctx.config.context_file_name}", - f"/{agent_path.name}/{PROJECT_MEMORY_PATH}", + f"/{agent_path.name}/{DAIV_MEMORY_PATH}", ], ), AnthropicPromptCachingMiddleware(), diff --git a/daiv/automation/agent/skills/creating-agents-md-file/SKILL.md b/daiv/automation/agent/skills/agentsmd-creation/SKILL.md similarity index 97% rename from daiv/automation/agent/skills/creating-agents-md-file/SKILL.md rename to daiv/automation/agent/skills/agentsmd-creation/SKILL.md index f7248a62..4bea03f9 100644 --- a/daiv/automation/agent/skills/creating-agents-md-file/SKILL.md +++ b/daiv/automation/agent/skills/agentsmd-creation/SKILL.md @@ -1,10 +1,8 @@ --- -name: creating-agents-md-file +name: agentsmd-creation description: Generate or update an AGENTS.md file by analyzing a repository's structure, commands, tests, and conventions. Use when asked to create or improve `AGENTS.md`. --- -# Creating AGENTS.md - Please analyze this codebase and create an `AGENTS.md` file, which will be given to future instances of DAIV and other AI agents to operate in this repository. diff --git a/daiv/automation/agent/skills/code-review/SKILL.md b/daiv/automation/agent/skills/code-review/SKILL.md new file mode 100644 index 00000000..75d553d9 --- /dev/null +++ b/daiv/automation/agent/skills/code-review/SKILL.md @@ -0,0 +1,32 @@ +--- +name: code-review +description: Review code changes and provide structured feedback for merge/pull requests or diffs. Use when asked to review PR/MR changes, assess correctness, style, tests, performance, or security, and return actionable review notes. +--- + +# Code Review + +## Establish scope and inputs + +- Identify whether the request targets a merge/pull request, a local diff, or specific files. +- If a merge/pull request is referenced and the git platform tool is available, fetch context and diffs before reviewing: + - `gitlab("project-merge-request get --iid ", output_mode="detailed")` + - `gitlab("project-merge-request-diff list --mr-iid ")` + - `gitlab("project-merge-request-diff get --mr-iid --id ")` +- If a diff is already provided, review that directly without re-fetching. +- If the scope is ambiguous, infer it from the conversation history and available artifacts. + +## Review checklist + +- Validate correctness, edge cases, and error handling. +- Confirm adherence to project conventions and architecture. +- Check performance implications or scalability risks. +- Evaluate tests: coverage for new/changed behavior, missing tests, or flaky patterns. +- Highlight security considerations (input validation, authz/authn, secrets, data handling). +- Note documentation or changelog impacts when user-facing behavior changes. + +## Response format + +- **Overview**: 1-3 bullets on what changed. +- **Findings**: concise bullets grouped by severity (High/Medium/Low) with actionable fixes. +- **Suggestions**: optional improvements that are not blocking. +- **Tests**: what was run, what should be run, or gaps to cover. diff --git a/daiv/automation/agent/skills/security-audit/SKILL.md b/daiv/automation/agent/skills/security-audit/SKILL.md new file mode 100644 index 00000000..a688897f --- /dev/null +++ b/daiv/automation/agent/skills/security-audit/SKILL.md @@ -0,0 +1,34 @@ +--- +name: security-audit +description: Perform a security audit of code changes or related code paths. Use when asked to assess security risks in a PR/MR, issue, or feature area and report findings with severity and remediation. +--- + +# Security Audit + +## Establish scope and inputs + +- Determine whether the audit targets a merge/pull request, specific files, or a broader codebase area. +- If a merge/pull request is referenced and the git platform tool is available, fetch context and diffs before auditing: + - `gitlab("project-merge-request get --iid ", output_mode="detailed")` + - `gitlab("project-merge-request-diff list --mr-iid ")` + - `gitlab("project-merge-request-diff get --mr-iid --id ")` +- If a diff or file list is already provided, proceed without re-fetching. +- Scope the audit to the affected code paths and any critical adjacent components. + +## Audit checklist + +- Authentication and authorization correctness, including privilege boundaries. +- Input validation and injection risks (SQLi, XSS, command injection, SSRF). +- Secrets management (hardcoded tokens, leaked credentials, unsafe logging). +- Data protection (encryption at rest/in transit, PII handling, data minimization). +- Dependency and supply-chain risks (unsafe or outdated libraries). +- Error handling that may leak sensitive details. +- Cryptography usage (weak algorithms, insecure randomness, misuse). +- API security (rate limiting, CORS, authentication on endpoints). + +## Response format + +- **Summary**: 1-3 bullets on overall posture and hotspots. +- **Findings**: group by severity (Critical/High/Medium/Low) with clear remediation. +- **Recommendations**: non-blocking improvements and follow-ups. +- **Tests/Validation**: security tests to run or missing coverage. diff --git a/daiv/automation/agent/skills/skill-creator/SKILL.md b/daiv/automation/agent/skills/skill-creator/SKILL.md index 2cbddeb6..60530c26 100644 --- a/daiv/automation/agent/skills/skill-creator/SKILL.md +++ b/daiv/automation/agent/skills/skill-creator/SKILL.md @@ -1,6 +1,6 @@ --- name: skill-creator -description: "Guide for creating effective skills that extend DAIV agent with specialized knowledge, workflows, or tool integrations. Use this skill when the user wants to create a new skill, update an existing skill, or get guidance on skill design patterns." +description: Guide for creating effective skills that extend DAIV agent with specialized knowledge, workflows, or tool integrations. Use this skill when the user wants to create a new skill, update an existing skill, or get guidance on skill design patterns. --- # Skill Creator diff --git a/daiv/automation/agent/utils.py b/daiv/automation/agent/utils.py index ba845f8e..784e297f 100644 --- a/daiv/automation/agent/utils.py +++ b/daiv/automation/agent/utils.py @@ -228,3 +228,20 @@ def get_daiv_agent_kwargs(*, model_config: AgentModelConfig, use_max: bool = Fal thinking_level = settings.MAX_THINKING_LEVEL return {"model_names": [model] + fallback_models, "thinking_level": thinking_level} + + +def extract_body_from_frontmatter(frontmatter_text: str) -> str: + """ + Extract prompt from text. + + Args: + frontmatter_text (str): The frontmatter text to extract content from. + + Returns: + str: The extracted content. + """ + frontmatter_pattern = r"^---\s*\n(.*?)\n---\s*\n" + match = re.match(frontmatter_pattern, frontmatter_text, re.DOTALL) + if not match: + return frontmatter_text + return frontmatter_text[match.end() :] diff --git a/daiv/chat/api/utils.py b/daiv/chat/api/utils.py index ec1c2364..9e644ad8 100644 --- a/daiv/chat/api/utils.py +++ b/daiv/chat/api/utils.py @@ -8,6 +8,7 @@ from automation.agent.graph import create_daiv_agent from chat.api.schemas import ChatCompletionChunk +from codebase.base import Scope from codebase.context import set_runtime_ctx if TYPE_CHECKING: @@ -57,7 +58,7 @@ async def generate_stream( chunk_uuid = str(uuid.uuid4()) created = int(datetime.now().timestamp()) - async with set_runtime_ctx(repo_id=repo_id, ref=ref) as runtime_ctx: + async with set_runtime_ctx(repo_id=repo_id, scope=Scope.GLOBAL, ref=ref) as runtime_ctx: try: daiv_agent = await create_daiv_agent(ctx=runtime_ctx) diff --git a/daiv/chat/api/views.py b/daiv/chat/api/views.py index 1e1c3fff..280a26e0 100644 --- a/daiv/chat/api/views.py +++ b/daiv/chat/api/views.py @@ -9,6 +9,7 @@ from automation.agent.graph import create_daiv_agent from automation.agent.utils import extract_text_content +from codebase.base import Scope from codebase.context import set_runtime_ctx from core.constants import BOT_NAME @@ -56,7 +57,7 @@ async def create_chat_completion(request: HttpRequest, payload: ChatCompletionRe content_type="text/event-stream", ) try: - async with set_runtime_ctx(repo_id=repo_id, ref=ref) as runtime_ctx: + async with set_runtime_ctx(repo_id=repo_id, scope=Scope.GLOBAL, ref=ref) as runtime_ctx: daiv_agent = await create_daiv_agent(ctx=runtime_ctx) result = await daiv_agent.ainvoke(input_data, config=config, context=runtime_ctx) diff --git a/daiv/codebase/base.py b/daiv/codebase/base.py index f4592e66..aaa3436d 100644 --- a/daiv/codebase/base.py +++ b/daiv/codebase/base.py @@ -14,6 +14,21 @@ class GitPlatform(StrEnum): SWE = "swe" +class Scope(StrEnum): + """ + Scope of the conversation. + """ + + GLOBAL = "Global" + """The conversation is scoped to the global scope.""" + + ISSUE = "Issue" + """The conversation is scoped to an issue.""" + + MERGE_REQUEST = "Merge Request" + """The conversation is scoped to a merge request.""" + + class Repository(BaseModel): pk: int slug: str diff --git a/daiv/codebase/clients/github/api/callbacks.py b/daiv/codebase/clients/github/api/callbacks.py index d8ad6f23..402f0b91 100644 --- a/daiv/codebase/clients/github/api/callbacks.py +++ b/daiv/codebase/clients/github/api/callbacks.py @@ -8,10 +8,6 @@ from codebase.repo_config import RepositoryConfig from codebase.tasks import address_issue_task, address_mr_comments_task, address_mr_review_task from codebase.utils import note_mentions_daiv -from quick_actions.base import Scope -from quick_actions.parser import QuickActionCommand, parse_quick_action -from quick_actions.registry import quick_action_registry -from quick_actions.tasks import execute_issue_task, execute_merge_request_task from .models import Comment, Issue, PullRequest, Repository, Review # noqa: TC001 @@ -51,7 +47,7 @@ async def process_callback(self): class IssueCommentCallback(GitHubCallback): """ - GitHub Note Webhook for automatically address the review feedback on an pull request or process quick actions. + GitHub Note Webhook for automatically address the review feedback on an pull request or process slash commands. """ action: Literal["created", "edited", "deleted"] @@ -73,37 +69,13 @@ def accept_callback(self) -> bool: ): return False - return bool(self._is_quick_action or self._is_merge_request_review) + return bool(self._is_issue_comment or self._is_merge_request_review) async def process_callback(self): """ Trigger the task to address the review feedback or issue comment like the plan approval use case. """ - if self._is_quick_action: - logger.info("Found quick action in note: '%s'", self._quick_action_command.raw) - - self._client.create_issue_note_emoji( - self.repository.full_name, self.issue.number, Emoji.THUMBSUP, self.comment.id - ) - - if self._action_scope == Scope.ISSUE: - await execute_issue_task.aenqueue( - repo_id=self.repository.full_name, - comment_id=self.comment.id, - action_command=self._quick_action_command.command, - action_args=" ".join(self._quick_action_command.args), - issue_id=self.issue.number, - ) - elif self._action_scope == Scope.MERGE_REQUEST: - await execute_merge_request_task.aenqueue( - repo_id=self.repository.full_name, - comment_id=self.comment.id, - action_command=self._quick_action_command.command, - action_args=" ".join(self._quick_action_command.args), - merge_request_id=self.issue.number, - ) - - elif self._is_issue_comment: + if self._is_issue_comment: self._client.create_issue_note_emoji( self.repository.full_name, self.issue.number, Emoji.EYES, self.comment.id ) @@ -121,13 +93,6 @@ async def process_callback(self): merge_request_source_branch=merge_request.source_branch, ) - @property - def _is_quick_action(self) -> bool: - """ - Accept the webhook if the note is a quick action. - """ - return bool(self._repo_config.quick_actions.enabled and self._quick_action_command) - @property def _is_merge_request_review(self) -> bool: """ @@ -155,48 +120,6 @@ def _is_issue_comment(self) -> bool: and note_mentions_daiv(self.comment.body, self._client.current_user) ) - @cached_property - def _quick_action_command(self) -> QuickActionCommand | None: - """ - Get the quick action command from the note body. - """ - quick_action_command = parse_quick_action(self.comment.body, self._client.current_user.username) - - logger.debug("GitHub quick action command: %s", quick_action_command) - - if not quick_action_command: - return None - - action_classes = quick_action_registry.get_actions( - command=quick_action_command.command, scope=self._action_scope - ) - - if not action_classes: - logger.warning( - "Quick action '%s' not found in registry for scope '%s'", - quick_action_command.command, - self._action_scope, - ) - return None - - if len(action_classes) > 1: - logger.warning( - "Multiple quick actions found for '%s' in registry for scope '%s': %s", - quick_action_command.command, - self._action_scope, - [a.command for a in action_classes], - ) - return None - - return quick_action_command - - @property - def _action_scope(self) -> Scope: - """ - Get the scope of the quick action. - """ - return Scope.MERGE_REQUEST if self.issue.is_pull_request() else Scope.ISSUE - class PullRequestReviewCallback(GitHubCallback): """ diff --git a/daiv/codebase/clients/gitlab/api/callbacks.py b/daiv/codebase/clients/gitlab/api/callbacks.py index 353e4ce0..3d78ee16 100644 --- a/daiv/codebase/clients/gitlab/api/callbacks.py +++ b/daiv/codebase/clients/gitlab/api/callbacks.py @@ -9,10 +9,6 @@ from codebase.repo_config import RepositoryConfig from codebase.tasks import address_issue_task, address_mr_comments_task, address_mr_review_task from codebase.utils import note_mentions_daiv -from quick_actions.base import Scope -from quick_actions.parser import QuickActionCommand, parse_quick_action -from quick_actions.registry import quick_action_registry -from quick_actions.tasks import execute_issue_task, execute_merge_request_task from .models import Issue, IssueAction, MergeRequest, Note, NoteableType, NoteAction, Project, User @@ -26,7 +22,6 @@ class IssueCallback(BaseCallback): object_kind: Literal["issue", "work_item"] project: Project - user: User object_attributes: Issue def accept_callback(self) -> bool: @@ -78,42 +73,15 @@ def accept_callback(self) -> bool: ): return False - return bool(self._is_quick_action or self._is_issue_comment or self._is_merge_request_review) + return bool(self._is_issue_comment or self._is_merge_request_review) async def process_callback(self): """ - Trigger the task to address the review feedback, issue comment or quick action. + Trigger the task to address the review feedback, issue comment or slash command. GitLab Note Webhook is called multiple times, one per note/discussion. """ - if self._is_quick_action: - logger.info("Found quick action in note: '%s'", self._quick_action_command.raw) - - # Add a thumbsup emoji to the note to show the user that the quick action will be executed. - if self._action_scope == Scope.MERGE_REQUEST: - self._client.create_merge_request_note_emoji( - self.project.path_with_namespace, self.merge_request.iid, Emoji.EYES, self.object_attributes.id - ) - await execute_merge_request_task.aenqueue( - repo_id=self.project.path_with_namespace, - comment_id=self.object_attributes.discussion_id, - action_command=self._quick_action_command.command, - action_args=" ".join(self._quick_action_command.args), - merge_request_id=self.merge_request.iid, - ) - elif self._action_scope == Scope.ISSUE: - self._client.create_issue_note_emoji( - self.project.path_with_namespace, self.issue.iid, Emoji.EYES, self.object_attributes.id - ) - await execute_issue_task.aenqueue( - repo_id=self.project.path_with_namespace, - comment_id=self.object_attributes.discussion_id, - action_command=self._quick_action_command.command, - action_args=" ".join(self._quick_action_command.args), - issue_id=self.issue.iid, - ) - - elif self._is_issue_comment: + if self._is_issue_comment: self._client.create_issue_note_emoji( self.project.path_with_namespace, self.issue.iid, Emoji.EYES, self.object_attributes.id ) @@ -140,13 +108,6 @@ async def process_callback(self): else: logger.warning("Unsupported note type: %s", self.object_attributes.type) - @property - def _is_quick_action(self) -> bool: - """ - Accept the webhook if the note is a quick action. - """ - return bool(self._repo_config.quick_actions.enabled and self._quick_action_command) - @cached_property def _is_merge_request_review(self) -> bool: """ @@ -175,46 +136,6 @@ def _is_issue_comment(self) -> bool: and note_mentions_daiv(self.object_attributes.note, self._client.current_user) ) - @cached_property - def _quick_action_command(self) -> QuickActionCommand | None: - """ - Get the quick action command from the note body. - """ - quick_action_command = parse_quick_action(self.object_attributes.note, self._client.current_user.username) - - if not quick_action_command: - return None - - action_classes = quick_action_registry.get_actions( - command=quick_action_command.command, scope=self._action_scope - ) - - if not action_classes: - logger.warning( - "Quick action '%s' not found in registry for scope '%s'", - quick_action_command.command, - self._action_scope, - ) - return None - - if len(action_classes) > 1: - logger.warning( - "Multiple quick actions found for '%s' in registry for scope '%s': %s", - quick_action_command.command, - self._action_scope, - [a.command for a in action_classes], - ) - return None - - return quick_action_command - - @property - def _action_scope(self) -> Scope: - """ - Get the scope of the quick action. - """ - return Scope.ISSUE if self.object_attributes.noteable_type == NoteableType.ISSUE else Scope.MERGE_REQUEST - class PushCallback(BaseCallback): """ diff --git a/daiv/codebase/context.py b/daiv/codebase/context.py index 796a129e..92ae29eb 100644 --- a/daiv/codebase/context.py +++ b/daiv/codebase/context.py @@ -1,11 +1,11 @@ from contextlib import asynccontextmanager from contextvars import ContextVar from dataclasses import dataclass -from typing import TYPE_CHECKING, Any, Literal, cast +from typing import TYPE_CHECKING, Any, cast from git import Repo # noqa: TC002 -from codebase.base import GitPlatform, Issue, MergeRequest # noqa: TC001 +from codebase.base import GitPlatform, Issue, MergeRequest, Scope # noqa: TC001 from codebase.clients import RepoClient from codebase.repo_config import RepositoryConfig @@ -37,7 +37,7 @@ class RuntimeCtx: config: RepositoryConfig """The repository configuration""" - scope: Literal["issue", "merge_request"] | None = None + scope: Scope | None = None """The scope of the context. If None, not running in a specific scope.""" issue: Issue | None = None @@ -57,8 +57,8 @@ class RuntimeCtx: async def set_runtime_ctx( repo_id: str, *, + scope: Scope, ref: str | None = None, - scope: Literal["issue", "merge_request"] | None = None, issue: Issue | None = None, merge_request: MergeRequest | None = None, offline: bool = False, @@ -69,8 +69,10 @@ async def set_runtime_ctx( Args: repo_id: The repository identifier + scope: The scope of the context. ref: The reference branch or tag. If None, the default branch will be used. - scope: The scope of the context. If None, not running in a specific scope. + issue: The issue object if the context is scoped to an issue, None otherwise + merge_request: The merge request object if the context is scoped to a merge request, None otherwise offline: Whether to use the cached configuration or to fetch it from the repository. **kwargs: Additional keyword arguments to pass to the repository client. diff --git a/daiv/codebase/repo_config.py b/daiv/codebase/repo_config.py index e1c07570..4464e876 100644 --- a/daiv/codebase/repo_config.py +++ b/daiv/codebase/repo_config.py @@ -43,12 +43,12 @@ class CodeReview(BaseModel): enabled: bool = Field(default=True, description="Enable code review features.") -class QuickActions(BaseModel): +class SlashCommands(BaseModel): """ - Quick actions configuration. + Slash commands configuration. """ - enabled: bool = Field(default=True, description="Enable quick actions features.") + enabled: bool = Field(default=True, description="Enable slash command features.") class Sandbox(BaseModel): @@ -162,7 +162,9 @@ class RepositoryConfig(BaseModel): ) # Features - quick_actions: QuickActions = Field(default_factory=QuickActions, description="Configure quick actions features.") + slash_commands: SlashCommands = Field( + default_factory=SlashCommands, description="Configure slash command features." + ) code_review: CodeReview = Field(default_factory=CodeReview, description="Configure code review features.") issue_addressing: IssueAddressing = Field( default_factory=IssueAddressing, description="Configure issue addressing features." diff --git a/daiv/codebase/tasks.py b/daiv/codebase/tasks.py index c6068be1..b6bff35f 100644 --- a/daiv/codebase/tasks.py +++ b/daiv/codebase/tasks.py @@ -6,7 +6,7 @@ from crontask import cron -from codebase.base import GitPlatform +from codebase.base import GitPlatform, Scope from codebase.clients import RepoClient from codebase.conf import settings as codebase_settings from codebase.context import set_runtime_ctx @@ -44,7 +44,7 @@ async def address_issue_task( """ client = RepoClient.create_instance() issue = client.get_issue(repo_id, issue_iid) - async with set_runtime_ctx(repo_id, ref=ref, scope="issue", issue=issue) as runtime_ctx: + async with set_runtime_ctx(repo_id, scope=Scope.ISSUE, ref=ref, issue=issue) as runtime_ctx: await IssueAddressorManager.address_issue( issue=issue, mention_comment_id=mention_comment_id, runtime_ctx=runtime_ctx ) @@ -82,7 +82,7 @@ async def address_mr_comments_task( client = RepoClient.create_instance() merge_request = client.get_merge_request(repo_id, merge_request_id) async with set_runtime_ctx( - repo_id, ref=merge_request_source_branch, scope="merge_request", merge_request=merge_request + repo_id, scope=Scope.MERGE_REQUEST, ref=merge_request_source_branch, merge_request=merge_request ) as runtime_ctx: await CommentsAddressorManager.address_comments( merge_request=merge_request, mention_comment_id=mention_comment_id, runtime_ctx=runtime_ctx diff --git a/daiv/daiv/settings/components/common.py b/daiv/daiv/settings/components/common.py index bf41aaae..199f8191 100644 --- a/daiv/daiv/settings/components/common.py +++ b/daiv/daiv/settings/components/common.py @@ -9,7 +9,7 @@ # Application definition -LOCAL_APPS = ["accounts", "automation", "codebase", "core", "quick_actions"] +LOCAL_APPS = ["accounts", "automation", "codebase", "core", "slash_commands"] THIRD_PARTY_APPS = ["crontask", "django_extensions", "django_tasks", "django_tasks.backends.database"] diff --git a/daiv/quick_actions/actions/__init__.py b/daiv/quick_actions/actions/__init__.py deleted file mode 100644 index cfa8a098..00000000 --- a/daiv/quick_actions/actions/__init__.py +++ /dev/null @@ -1,4 +0,0 @@ -from .clone_to_topic import CloneToTopicQuickAction -from .help import HelpQuickAction - -__all__ = ["CloneToTopicQuickAction", "HelpQuickAction"] diff --git a/daiv/quick_actions/actions/clone_to_topic.py b/daiv/quick_actions/actions/clone_to_topic.py deleted file mode 100644 index 7d9037a7..00000000 --- a/daiv/quick_actions/actions/clone_to_topic.py +++ /dev/null @@ -1,72 +0,0 @@ -from contextlib import suppress -from typing import TYPE_CHECKING - -from django.template.loader import render_to_string - -from quick_actions.base import QuickAction, Scope -from quick_actions.decorator import quick_action - -if TYPE_CHECKING: - from codebase.base import Discussion, Issue - - -@quick_action(command="clone-to-topic", scopes=[Scope.ISSUE]) -class CloneToTopicQuickAction(QuickAction): - """ - Command to clone an issue to all repositories matching specified topics. - """ - - description: str = "Clone this issue to all repositories matching the specified topics." - - def validate_arguments(self, args: str) -> bool: - """ - Validate that topics are provided. - - Args: - args: The arguments to validate. - - Returns: - True if topics are provided, False otherwise. - """ - return bool(args.strip()) - - async def execute_action_for_issue(self, repo_id: str, *, args: str, comment: Discussion, issue: Issue) -> None: - """ - Clone the issue to all repositories matching the specified topics. - - Args: - repo_id: The repository ID. - comment: The comment that triggered the action. - issue: The issue where the action was triggered. - args: Comma-separated list of topics. - """ - # Parse topics from args - topics = [topic.strip() for topic in args.split(",") if topic.strip()] - - if not topics: - self._add_invalid_args_message(repo_id, issue.iid, comment.id, args, scope=Scope.ISSUE) - return - - target_repos = [repo for repo in self.client.list_repositories(topics=topics) if repo.slug != repo_id] - - if not target_repos: - topics_str = ", ".join([f"`{topic}`" for topic in topics]) - message = f"No repositories matching the specified topics {topics_str} found." - self.client.create_issue_comment(repo_id, issue.iid, message, reply_to_id=comment.id) - return - - cloned_issues = [] - - for target_repo in target_repos: - with suppress(Exception): - cloned_issue_iid = self.client.create_issue( - repo_id=target_repo.slug, title=issue.title, description=issue.description, labels=issue.labels - ) - - cloned_issues.append(f"{target_repo.slug}#{cloned_issue_iid}") - - note_message = render_to_string( - "quick_actions/clone_to_topic_result.txt", - {"total_count": len(cloned_issues), "cloned_issues": cloned_issues}, - ) - self.client.create_issue_comment(repo_id, issue.iid, note_message, reply_to_id=comment.id) diff --git a/daiv/quick_actions/actions/help.py b/daiv/quick_actions/actions/help.py deleted file mode 100644 index 095c12f2..00000000 --- a/daiv/quick_actions/actions/help.py +++ /dev/null @@ -1,60 +0,0 @@ -from typing import TYPE_CHECKING - -from django.template.loader import render_to_string - -from core.constants import BOT_NAME -from quick_actions.base import QuickAction, Scope -from quick_actions.decorator import quick_action -from quick_actions.registry import quick_action_registry - -if TYPE_CHECKING: - from codebase.base import Discussion, Issue, MergeRequest - - -@quick_action(command="help", scopes=[Scope.ISSUE, Scope.MERGE_REQUEST]) -class HelpQuickAction(QuickAction): - """ - Shows the help message for the available quick actions. - """ - - description: str = "Shows the help message with the available quick actions." - - async def execute_action_for_issue(self, repo_id: str, *, args: str, comment: Discussion, issue: Issue) -> None: - """ - Execute the help action. - - Args: - repo_id: The repository ID. - args: Additional parameters from the command. - comment: The comment that triggered the action. - issue: The issue where the action was triggered (if applicable). - """ - if note_message := self._get_note_message(Scope.ISSUE): - self.client.create_issue_comment(repo_id, issue.iid, note_message) - - async def execute_action_for_merge_request( - self, repo_id: str, *, args: str, comment: Discussion, merge_request: MergeRequest - ) -> None: - """ - Execute the help action. - - Args: - repo_id: The repository ID. - args: Additional parameters from the command. - comment: The comment that triggered the action. - merge_request: The merge request where the action was triggered (if applicable). - """ - if note_message := self._get_note_message(Scope.MERGE_REQUEST): - self.client.create_merge_request_comment(repo_id, merge_request.merge_request_id, note_message) - - def _get_note_message(self, scope: Scope) -> str | None: - """ - Get the note message for the given scope. - """ - actions_help = [action().help() for action in quick_action_registry.get_actions(scope=scope)] - if not actions_help: - return None - return render_to_string( - "quick_actions/quick_actions_help.txt", - {"bot_name": BOT_NAME, "scope": scope.value.lower(), "actions": actions_help}, - ) diff --git a/daiv/quick_actions/base.py b/daiv/quick_actions/base.py deleted file mode 100644 index 3f05fc5f..00000000 --- a/daiv/quick_actions/base.py +++ /dev/null @@ -1,138 +0,0 @@ -from __future__ import annotations - -from abc import ABC -from enum import StrEnum -from typing import TYPE_CHECKING - -from django.template.loader import render_to_string - -from codebase.clients import RepoClient - -if TYPE_CHECKING: - from codebase.base import Discussion, Issue, MergeRequest - - -class Scope(StrEnum): - ISSUE = "Issue" - MERGE_REQUEST = "Merge Request" - - -class QuickAction(ABC): - """ - Base class for quick actions. - """ - - command: str - scopes: list[Scope] - description: str - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.client = RepoClient.create_instance() - - def help(self) -> str: - """ - Get the help message for the action. - """ - return f" * `{self.command_to_activate}` - {self.description}" - - @property - def command_to_activate(self) -> str: - """ - Get the command to activate the action. - """ - return f"@{self.client.current_user.username} /{self.command}" - - async def execute_for_issue(self, repo_id: str, *, args: str, comment: Discussion, issue: Issue) -> None: - """ - Execute the quick action for an issue. - """ - if not self.validate_arguments(args): - self._add_invalid_args_message(repo_id, issue.iid, comment.id, args, scope=Scope.ISSUE) - return - - return await self.execute_action_for_issue(repo_id, args=args, comment=comment, issue=issue) - - async def execute_for_merge_request( - self, repo_id: str, *, args: str, comment: Discussion, merge_request: MergeRequest - ) -> None: - """ - Execute the quick action for a merge request. - """ - if not self.validate_arguments(args): - self._add_invalid_args_message( - repo_id, merge_request.merge_request_id, comment.id, args, scope=Scope.MERGE_REQUEST - ) - return - - return await self.execute_action_for_merge_request( - repo_id, args=args, comment=comment, merge_request=merge_request - ) - - async def execute_action_for_issue(self, repo_id: str, *, args: str, comment: Discussion, issue: Issue) -> None: - """ - Use this method to implement the specific logic for the action to be executed. - - Args: - repo_id: The repository ID. - comment: The comment that triggered the action. - issue: The issue where the action was triggered (if applicable). - args: Additional parameters from the command. - """ - raise NotImplementedError("execute_action_for_issue is not implemented") - - async def execute_action_for_merge_request( - self, repo_id: str, *, args: str, comment: Discussion, merge_request: MergeRequest - ) -> None: - """ - Execute the quick action for a merge request. - - Args: - repo_id: The repository ID. - comment: The comment that triggered the action. - merge_request: The merge request where the action was triggered (if applicable). - args: Additional parameters from the command. - """ - raise NotImplementedError("execute_action_for_merge_request is not implemented") - - def validate_arguments(self, args: str) -> bool: - """ - Validate the arguments are valid. - - Args: - args: The arguments to validate. - - Returns: - bool: True if the arguments are valid, False otherwise. - """ - return True - - def _add_invalid_args_message( - self, repo_id: str, object_id: int, comment_id: str, invalid_args: str, scope: Scope - ) -> None: - """ - Add an invalid arguments message to the merge request discussion. - - Args: - repo_id: The repository ID. - object_id: The merge request or issue ID. - comment_id: The comment ID of the note. - invalid_args: The invalid arguments. - scope: The scope of the quick action. - """ - note_message = render_to_string( - "quick_actions/invalid_args.txt", - { - "bot_name": self.client.current_user.username, - "command": self.command, - "help": self.help(), - "invalid_args": invalid_args, - }, - ) - - if scope == Scope.MERGE_REQUEST: - self.client.create_merge_request_comment( - repo_id, object_id, note_message, reply_to_id=comment_id, mark_as_resolved=True - ) - elif scope == Scope.ISSUE: - self.client.create_issue_comment(repo_id, object_id, note_message, reply_to_id=comment_id) diff --git a/daiv/quick_actions/decorator.py b/daiv/quick_actions/decorator.py deleted file mode 100644 index 2dc8bfdb..00000000 --- a/daiv/quick_actions/decorator.py +++ /dev/null @@ -1,31 +0,0 @@ -from typing import TYPE_CHECKING - -from .registry import quick_action_registry - -if TYPE_CHECKING: - from collections.abc import Callable - - from .base import QuickAction, Scope - - -def quick_action(command: str, scopes: list[Scope]) -> Callable[[type[QuickAction]], type[QuickAction]]: - """ - Decorator to register a quick action. - - Usage: - @quick_action(command="my_action", scopes=[Scopes.ISSUE, Scopes.MERGE_REQUEST]) - class MyAction(QuickAction): - # ... implementation - - Args: - cls: The quick action class to register. - - Returns: - The quick action class. - """ - - def decorator(cls: type[QuickAction]) -> type[QuickAction]: - quick_action_registry.register(cls, command, scopes) - return cls - - return decorator diff --git a/daiv/quick_actions/parser.py b/daiv/quick_actions/parser.py deleted file mode 100644 index d46107fb..00000000 --- a/daiv/quick_actions/parser.py +++ /dev/null @@ -1,53 +0,0 @@ -import re -import shlex -from dataclasses import dataclass - - -@dataclass -class QuickActionCommand: - """ - Structured result of a parsed bot command. - """ - - command: str - args: list[str] - raw: str - - -_COMMAND_RE_TEMPLATE = r""" - (?{bot}) # literal @bot-name mention - \s+ # at least one space or tab - / # literal slash before command - (?P[^\n\r]+) # capture the rest of the line (until newline) -""" - - -def parse_quick_action(note_body: str, bot_name: str) -> QuickActionCommand | None: - """ - Parse the first '@ …' command in `note_body`. - - Args: - note_body: The full text of a GitLab note / comment. - bot_name: The bot mention to look for (case-insensitive). - - Returns: - QuickActionCommand if found, otherwise None. - """ - pattern = _COMMAND_RE_TEMPLATE.format(bot=re.escape(bot_name)) - - match = re.search(pattern, note_body, flags=re.IGNORECASE | re.VERBOSE) - if not match: - return None - - raw_line = match.group(0).strip() - try: - parts = shlex.split(match.group("cmd")) - except ValueError: - return None - - if not parts: - return None - - command, *args = parts - return QuickActionCommand(command=command.lower(), args=args, raw=raw_line) diff --git a/daiv/quick_actions/registry.py b/daiv/quick_actions/registry.py deleted file mode 100644 index e88da12a..00000000 --- a/daiv/quick_actions/registry.py +++ /dev/null @@ -1,63 +0,0 @@ -from __future__ import annotations - -from inspect import isclass - -from .base import QuickAction, Scope - - -class QuickActionRegistry: - """ - Registry that keeps track of the registered quick actions. - """ - - def __init__(self): - self._registry: dict[str, type[QuickAction]] = {} - self._registry_by_scope: dict[str, list[type[QuickAction]]] = {} - - def register(self, action: type[QuickAction], command: str, scopes: list[Scope]) -> None: - """ - Register a quick action class. - - Args: - action: The quick action class to register. - command: The command to register the action with. - scopes: The scopes to register the action for. - """ - assert isclass(action) and issubclass(action, QuickAction), ( - f"{action} must be a class that inherits from QuickAction" - ) - assert action not in self._registry.values(), f"{action.__name__} is already registered as quick action." - assert command not in self._registry, f"{command} is already registered as quick action." - - action.command = command # type: ignore - action.scopes = scopes # type: ignore - - self._registry[command] = action - for scope in scopes: - if scope.value not in self._registry_by_scope: - self._registry_by_scope[scope.value] = [] - self._registry_by_scope[scope.value].append(action) - - def get_actions(self, scope: Scope | None = None, command: str | None = None) -> list[type[QuickAction]]: - """ - Get quick actions that support the given scope. - - Args: - scope: The scope to get quick actions for. - command: The command to get quick actions for. - - Returns: - List of quick action classes that support the given scope. - """ - if scope is None and command is None: - return list(self._registry.values()) - if scope is None: - if action := self._registry.get(command, None): - return [action] - return [] - if command is None: - return self._registry_by_scope.get(scope.value, []) - return list(filter(lambda x: x.command == command, self._registry_by_scope.get(scope.value, []))) - - -quick_action_registry = QuickActionRegistry() diff --git a/daiv/quick_actions/tasks.py b/daiv/quick_actions/tasks.py deleted file mode 100644 index fb32cf32..00000000 --- a/daiv/quick_actions/tasks.py +++ /dev/null @@ -1,118 +0,0 @@ -import logging - -from django.tasks import task -from django.template.loader import render_to_string - -from codebase.clients import RepoClient - -from .base import Scope -from .registry import quick_action_registry - -logger = logging.getLogger("daiv.quick_actions") - - -@task -async def execute_issue_task(repo_id: str, action_command: str, action_args: str, comment_id: str, issue_id: int): - """ - Execute a quick action asynchronously. - - Args: - repo_id: The repository ID. - action_command: The command of the quick action to execute. - action_args: Additional parameters from the command. - comment_id: The ID of the comment to execute the action on. - issue_id: The ID of the issue to execute the action on. - """ - action_classes = quick_action_registry.get_actions(command=action_command, scope=Scope.ISSUE) - - if not action_classes: - logger.error("Quick action '%s' not found in registry for scope '%s'", action_command, Scope.ISSUE) - return - - if len(action_classes) > 1: - logger.error( - "Multiple quick actions found for '%s' in registry for scope '%s': %s", - action_command, - Scope.ISSUE, - [a.command for a in action_classes], - ) - return - - client = RepoClient.create_instance() - - comment = client.get_issue_comment(repo_id, issue_id, comment_id) - issue = client.get_issue(repo_id, issue_id) - - try: - action = action_classes[0]() - await action.execute_for_issue(repo_id=repo_id, args=action_args, comment=comment, issue=issue) - except Exception as e: - logger.exception("Error executing quick action '%s' for repo '%s': %s", action_command, repo_id, str(e)) - - error_message = render_to_string( - "quick_actions/error_message.txt", - {"command": f"@{client.current_user.username} /{action_command} {action_args}".strip()}, - ) - - client.create_issue_comment(repo_id, issue_id, error_message) - else: - logger.info( - "Successfully executed quick action '%s' for repo '%s' on issue '%s'", action_command, repo_id, issue_id - ) - - -@task -async def execute_merge_request_task( - repo_id: str, action_command: str, action_args: str, comment_id: str, merge_request_id: int -) -> None: - """ - Execute a quick action asynchronously. - - Args: - repo_id: The repository ID. - action_command: The command of the quick action to execute. - action_args: Additional parameters from the command. - comment_id: The ID of the comment to execute the action on. - merge_request_id: The ID of the merge request to execute the action on (if applicable). - """ - action_classes = quick_action_registry.get_actions(command=action_command, scope=Scope.MERGE_REQUEST) - - if not action_classes: - logger.error("Quick action '%s' not found in registry for scope '%s'", action_command, Scope.MERGE_REQUEST) - return - - if len(action_classes) > 1: - logger.error( - "Multiple quick actions found for '%s' in registry for scope '%s': %s", - action_command, - Scope.MERGE_REQUEST, - [a.command for a in action_classes], - ) - return - - client = RepoClient.create_instance() - - comment = client.get_merge_request_comment(repo_id, merge_request_id, comment_id) - merge_request = client.get_merge_request(repo_id, merge_request_id) - - try: - action = action_classes[0]() - await action.execute_for_merge_request( - repo_id=repo_id, args=action_args, comment=comment, merge_request=merge_request - ) - except Exception as e: - logger.exception("Error executing quick action '%s' for repo '%s': %s", action_command, repo_id, str(e)) - - error_message = render_to_string( - "quick_actions/error_message.txt", - {"command": f"@{client.current_user.username} /{action_command} {action_args}".strip()}, - ) - - client.create_merge_request_comment(repo_id, merge_request.merge_request_id, error_message) - else: - logger.info( - "Successfully executed quick action '%s' for repo '%s' on merge request '%s'", - action_command, - repo_id, - merge_request.merge_request_id, - ) diff --git a/daiv/quick_actions/templates/quick_actions/invalid_args.txt b/daiv/quick_actions/templates/quick_actions/invalid_args.txt deleted file mode 100644 index 1864d88e..00000000 --- a/daiv/quick_actions/templates/quick_actions/invalid_args.txt +++ /dev/null @@ -1,10 +0,0 @@ -### ⚠️ Invalid Arguments for Quick-Action - -`@{{ bot_name }} /{{ command }} {{ invalid_args }}` aren't a recognised arguments for **/{{ command }}**. - -**Here's how to use it correctly:** - -{{ help }} - -Need more options? Comment **`@{{ bot_name }} help`** to see the full quick-action reference. - diff --git a/daiv/slash_commands/__init__.py b/daiv/slash_commands/__init__.py new file mode 100644 index 00000000..12659a8b --- /dev/null +++ b/daiv/slash_commands/__init__.py @@ -0,0 +1 @@ +"""Slash commands app for DAIV.""" diff --git a/daiv/slash_commands/actions/__init__.py b/daiv/slash_commands/actions/__init__.py new file mode 100644 index 00000000..7a69f4f4 --- /dev/null +++ b/daiv/slash_commands/actions/__init__.py @@ -0,0 +1,4 @@ +from .clone_to_topic import CloneToTopicSlashCommand +from .help import HelpSlashCommand + +__all__ = ["CloneToTopicSlashCommand", "HelpSlashCommand"] diff --git a/daiv/slash_commands/actions/clone_to_topic.py b/daiv/slash_commands/actions/clone_to_topic.py new file mode 100644 index 00000000..607eb6cc --- /dev/null +++ b/daiv/slash_commands/actions/clone_to_topic.py @@ -0,0 +1,124 @@ +from contextlib import suppress +from typing import TYPE_CHECKING + +from django.template.loader import render_to_string + +from codebase.base import Scope +from slash_commands.base import SlashCommand +from slash_commands.decorator import slash_command + +if TYPE_CHECKING: + from codebase.base import Discussion, Issue + + +@slash_command(command="clone-to-topic", scopes=[Scope.ISSUE]) +class CloneToTopicSlashCommand(SlashCommand): + """ + Command to clone an issue to all repositories matching specified topics. + """ + + description: str = "Clone this issue to all repositories matching the specified topics." + + def validate_arguments(self, args: str) -> bool: + """ + Validate that topics are provided. + + Args: + args: The arguments to validate. + + Returns: + True if topics are provided, False otherwise. + """ + return bool(args.strip()) + + async def execute_action_for_issue(self, repo_id: str, *, args: str, comment: Discussion, issue: Issue) -> None: + """ + Clone the issue to all repositories matching the specified topics. + + Args: + repo_id: The repository ID. + comment: The comment that triggered the command. + issue: The issue where the command was triggered. + args: Comma-separated list of topics. + """ + # Parse topics from args + topics = [topic.strip() for topic in args.split(",") if topic.strip()] + + if not topics: + self._add_invalid_args_message(repo_id, issue.iid, comment.id, args, scope=Scope.ISSUE) + return + + target_repos = [repo for repo in self.client.list_repositories(topics=topics) if repo.slug != repo_id] + + if not target_repos: + topics_str = ", ".join([f"`{topic}`" for topic in topics]) + message = f"No repositories matching the specified topics {topics_str} found." + self.client.create_issue_comment(repo_id, issue.iid, message, reply_to_id=comment.id) + return + + cloned_issues = [] + + for target_repo in target_repos: + with suppress(Exception): + cloned_issue_iid = self.client.create_issue( + repo_id=target_repo.slug, title=issue.title, description=issue.description, labels=issue.labels + ) + + cloned_issues.append(f"{target_repo.slug}#{cloned_issue_iid}") + + note_message = render_to_string( + "slash_commands/clone_to_topic_result.txt", + {"total_count": len(cloned_issues), "cloned_issues": cloned_issues}, + ) + self.client.create_issue_comment(repo_id, issue.iid, note_message, reply_to_id=comment.id) + + async def execute_for_agent( + self, + *, + args: str, + scope: Scope, + repo_id: str, + bot_username: str, + issue_iid: int | None = None, + merge_request_id: int | None = None, + ) -> str: + """ + Execute clone-to-topic command for agent middleware. + + Returns a message listing the cloned issues or an error message. + """ + if not self.validate_arguments(args): + return f"Invalid arguments for /{self.command}. Please provide comma-separated topics." + + if scope != Scope.ISSUE or issue_iid is None: + return f"The /{self.command} command is only available for issues." + + # Parse topics from args + topics = [topic.strip() for topic in args.split(",") if topic.strip()] + + if not topics: + return f"Invalid arguments for /{self.command}. Please provide comma-separated topics." + + # Get the issue details + issue = self.client.get_issue(repo_id, issue_iid) + + target_repos = [repo for repo in self.client.list_repositories(topics=topics) if repo.slug != repo_id] + + if not target_repos: + topics_str = ", ".join([f"`{topic}`" for topic in topics]) + return f"No repositories matching the specified topics {topics_str} found." + + cloned_issues = [] + + for target_repo in target_repos: + with suppress(Exception): + cloned_issue_iid = self.client.create_issue( + repo_id=target_repo.slug, title=issue.title, description=issue.description, labels=issue.labels + ) + + cloned_issues.append(f"{target_repo.slug}#{cloned_issue_iid}") + + return render_to_string( + "slash_commands/clone_to_topic_result.txt", + {"total_count": len(cloned_issues), "cloned_issues": cloned_issues}, + ) diff --git a/daiv/slash_commands/actions/help.py b/daiv/slash_commands/actions/help.py new file mode 100644 index 00000000..dfdcf7f6 --- /dev/null +++ b/daiv/slash_commands/actions/help.py @@ -0,0 +1,48 @@ +from django.template.loader import render_to_string + +from codebase.base import Scope +from core.constants import BOT_NAME +from slash_commands.base import SlashCommand +from slash_commands.decorator import slash_command +from slash_commands.registry import slash_command_registry + + +@slash_command(command="help", scopes=[Scope.GLOBAL, Scope.ISSUE, Scope.MERGE_REQUEST]) +class HelpSlashCommand(SlashCommand): + """ + Shows the help message for the available slash commands. + """ + + description: str = "Shows the help message with the available slash commands." + + async def execute_for_agent( + self, + *, + args: str, + scope: Scope, + repo_id: str, + bot_username: str, + issue_iid: int | None = None, + merge_request_id: int | None = None, + ) -> str: + """ + Execute help command for agent middleware. + + Args: + args: Additional parameters from the command. + scope: The scope to get the help message for. + repo_id: The repository ID. + bot_username: The bot username. + issue_iid: The issue IID (for Issue scope). + merge_request_id: The merge request ID (for Merge Request scope). + + Returns: + The help message for the given scope. + """ + commands_help = [command().help() for command in slash_command_registry.get_commands(scope=scope)] + if not commands_help: + return "No slash commands available." + return render_to_string( + "slash_commands/slash_commands_help.txt", + {"bot_name": BOT_NAME, "scope": scope.value.lower(), "actions": commands_help}, + ) diff --git a/daiv/quick_actions/apps.py b/daiv/slash_commands/apps.py similarity index 61% rename from daiv/quick_actions/apps.py rename to daiv/slash_commands/apps.py index fa25a8ba..958c2a8b 100644 --- a/daiv/quick_actions/apps.py +++ b/daiv/slash_commands/apps.py @@ -3,10 +3,10 @@ from django.utils.translation import gettext_lazy as _ -class QuickActionsConfig(AppConfig): - name = "quick_actions" - label = "quick_actions" - verbose_name = _("Quick Actions") +class SlashCommandsConfig(AppConfig): + name = "slash_commands" + label = "slash_commands" + verbose_name = _("Slash Commands") def ready(self): autodiscover_modules("actions") diff --git a/daiv/slash_commands/base.py b/daiv/slash_commands/base.py new file mode 100644 index 00000000..a6bdaff8 --- /dev/null +++ b/daiv/slash_commands/base.py @@ -0,0 +1,106 @@ +from __future__ import annotations + +from abc import ABC + +from django.template.loader import render_to_string + +from codebase.base import Scope +from codebase.clients import RepoClient + + +class SlashCommand(ABC): + """ + Base class for slash commands. + """ + + command: str + scopes: list[Scope] + description: str + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.client = RepoClient.create_instance() + + def help(self) -> str: + """ + Get the help message for the command. + """ + return f" * `{self.command_to_activate}` - {self.description}" + + @property + def command_to_activate(self) -> str: + """ + Get the command to activate the action. + """ + return f"@{self.client.current_user.username} /{self.command}" + + async def execute_for_agent( + self, + *, + args: str, + scope: Scope, + repo_id: str, + bot_username: str, + issue_iid: int | None = None, + merge_request_id: int | None = None, + ) -> str: + """ + Execute the slash command for agent middleware. + + This method should be implemented by subclasses to return a result content + instead of side-effecting via client calls. + + Args: + args: Additional parameters from the command. + scope: The scope (Issue or Merge Request). + repo_id: The repository ID. + bot_username: The bot username. + issue_iid: The issue IID (for Issue scope). + merge_request_id: The merge request ID (for Merge Request scope). + + Returns: + The result content. + """ + raise NotImplementedError("execute_for_agent is not implemented") + + def validate_arguments(self, args: str) -> bool: + """ + Validate the arguments are valid. + + Args: + args: The arguments to validate. + + Returns: + bool: True if the arguments are valid, False otherwise. + """ + return True + + def _add_invalid_args_message( + self, repo_id: str, object_id: int, comment_id: str, invalid_args: str, scope: Scope + ) -> None: + """ + Add an invalid arguments message to the merge request discussion. + + Args: + repo_id: The repository ID. + object_id: The merge request or issue ID. + comment_id: The comment ID of the note. + invalid_args: The invalid arguments. + scope: The scope of the slash command. + """ + note_message = render_to_string( + "slash_commands/invalid_args.txt", + { + "bot_name": self.client.current_user.username, + "command": self.command, + "help": self.help(), + "invalid_args": invalid_args, + }, + ) + + if scope == Scope.MERGE_REQUEST: + self.client.create_merge_request_comment( + repo_id, object_id, note_message, reply_to_id=comment_id, mark_as_resolved=True + ) + elif scope == Scope.ISSUE: + self.client.create_issue_comment(repo_id, object_id, note_message, reply_to_id=comment_id) diff --git a/daiv/slash_commands/decorator.py b/daiv/slash_commands/decorator.py new file mode 100644 index 00000000..e85f4aa8 --- /dev/null +++ b/daiv/slash_commands/decorator.py @@ -0,0 +1,36 @@ +from typing import TYPE_CHECKING + +from .registry import slash_command_registry + +if TYPE_CHECKING: + from collections.abc import Callable + + from codebase.base import Scope + + from .base import SlashCommand + + +def slash_command(command: str, scopes: list[Scope]) -> Callable[[type[SlashCommand]], type[SlashCommand]]: + """ + Decorator to register a slash command. + + Usage: + @slash_command(command="my_command", scopes=[Scopes.ISSUE, Scopes.MERGE_REQUEST]) + class MyCommand(SlashCommand): + # ... implementation + + Args: + cls: The slash command class to register. + + Returns: + The slash command class. + """ + + def decorator(cls: type[SlashCommand]) -> type[SlashCommand]: + slash_command_registry.register(cls, command, scopes) + return cls + + return decorator + + +__all__ = ["slash_command"] diff --git a/daiv/slash_commands/parser.py b/daiv/slash_commands/parser.py new file mode 100644 index 00000000..7863a91a --- /dev/null +++ b/daiv/slash_commands/parser.py @@ -0,0 +1,80 @@ +import re +import shlex +from dataclasses import dataclass + + +@dataclass +class SlashCommandCommand: + """ + Structured result of a parsed bot command. + """ + + command: str + args: list[str] + raw: str + + +_COMMAND_RE_TEMPLATE = r""" + (?{bot}) # literal @bot-name mention + \s+ # at least one space or tab + / # literal slash before command + (?P[^\n\r]+) # capture the rest of the line (until newline) +""" + + +def _parse_command_match(text: str, *, pattern: str, flags: int) -> SlashCommandCommand | None: + match = re.search(pattern, text, flags=flags) + if not match: + return None + + raw_line = match.group(0).strip() + try: + parts = shlex.split(match.group("cmd")) + except ValueError: + return None + + if not parts: + return None + + command, *args = parts + return SlashCommandCommand(command=command.lower(), args=args, raw=raw_line) + + +def parse_slash_command(note_body: str, bot_name: str) -> SlashCommandCommand | None: + """ + Parse the first '@ …' command in `note_body`. + + Args: + note_body: The full text of a GitLab note / comment. + bot_name: The bot mention to look for (case-insensitive). + + Returns: + SlashCommandCommand if found, otherwise None. + """ + pattern = _COMMAND_RE_TEMPLATE.format(bot=re.escape(bot_name)) + + return _parse_command_match(note_body, pattern=pattern, flags=re.IGNORECASE | re.VERBOSE) + + +def parse_agent_slash_command(text: str, bot_name: str) -> SlashCommandCommand | None: + """ + Parse slash commands for agent middleware. + + Supports both mention-based format (`@ /command ...`) and bare slash commands (`/command ...`). + + Args: + text: The message text to parse. + bot_name: The bot mention to look for (case-insensitive). + + Returns: + SlashCommandCommand if found, otherwise None. + """ + # Try mention-based format first + if result := parse_slash_command(text, bot_name): + return result + + # Try bare slash command format + # Look for lines starting with '/' (optionally preceded by whitespace) + bare_pattern = r"^\s*/(?P[^\n\r]+)" + return _parse_command_match(text, pattern=bare_pattern, flags=re.MULTILINE) diff --git a/daiv/slash_commands/registry.py b/daiv/slash_commands/registry.py new file mode 100644 index 00000000..345422c4 --- /dev/null +++ b/daiv/slash_commands/registry.py @@ -0,0 +1,67 @@ +from __future__ import annotations + +from inspect import isclass +from typing import TYPE_CHECKING + +from .base import SlashCommand + +if TYPE_CHECKING: + from codebase.base import Scope + + +class SlashCommandRegistry: + """ + Registry that keeps track of the registered slash commands. + """ + + def __init__(self): + self._registry: dict[str, type[SlashCommand]] = {} + self._registry_by_scope: dict[str, list[type[SlashCommand]]] = {} + + def register(self, command_cls: type[SlashCommand], command: str, scopes: list[Scope]) -> None: + """ + Register a slash command class. + + Args: + command_cls: The slash command class to register. + command: The command to register the action with. + scopes: The scopes to register the action for. + """ + assert isclass(command_cls) and issubclass(command_cls, SlashCommand), ( + f"{command_cls} must be a class that inherits from SlashCommand" + ) + assert command_cls not in self._registry.values(), f"{command_cls.__name__} is already registered." + assert command not in self._registry, f"{command} is already registered." + + command_cls.command = command # type: ignore + command_cls.scopes = scopes # type: ignore + + self._registry[command] = command_cls + for scope in scopes: + if scope.value not in self._registry_by_scope: + self._registry_by_scope[scope.value] = [] + self._registry_by_scope[scope.value].append(command_cls) + + def get_commands(self, scope: Scope | None = None, command: str | None = None) -> list[type[SlashCommand]]: + """ + Get slash commands that support the given scope. + + Args: + scope: The scope to get slash commands for. + command: The command to get slash commands for. + + Returns: + List of slash command classes that support the given scope. + """ + if scope is None and command is None: + return list(self._registry.values()) + if scope is None: + if command_cls := self._registry.get(command): + return [command_cls] + return [] + if command is None: + return self._registry_by_scope.get(scope.value, []) + return list(filter(lambda x: x.command == command, self._registry_by_scope.get(scope.value, []))) + + +slash_command_registry = SlashCommandRegistry() diff --git a/daiv/quick_actions/templates/quick_actions/clone_to_topic_result.txt b/daiv/slash_commands/templates/slash_commands/clone_to_topic_result.txt similarity index 100% rename from daiv/quick_actions/templates/quick_actions/clone_to_topic_result.txt rename to daiv/slash_commands/templates/slash_commands/clone_to_topic_result.txt diff --git a/daiv/quick_actions/templates/quick_actions/error_message.txt b/daiv/slash_commands/templates/slash_commands/error_message.txt similarity index 66% rename from daiv/quick_actions/templates/quick_actions/error_message.txt rename to daiv/slash_commands/templates/slash_commands/error_message.txt index 178caf96..ea79d256 100644 --- a/daiv/quick_actions/templates/quick_actions/error_message.txt +++ b/daiv/slash_commands/templates/slash_commands/error_message.txt @@ -1,9 +1,9 @@ -### ❌ Quick-Action Error +### ❌ Slash Command Error -I tried to run **`{{ command }}`**, but something unexpected happened and the action didn't complete. +I tried to run **`{{ command }}`**, but something unexpected happened and the command didn't complete. **What you can do now** -1. 🔄 **Retry** - simply add the same quick-action comment again. +1. 🔄 **Retry** - add the same slash command again. 2. 📜 **Check the app logs** - open the DAIV logs to see the full stack trace and [open an issue](https://github.com/srtab/daiv/issues/new) if the problem persists. diff --git a/daiv/slash_commands/templates/slash_commands/invalid_args.txt b/daiv/slash_commands/templates/slash_commands/invalid_args.txt new file mode 100644 index 00000000..8a5343ba --- /dev/null +++ b/daiv/slash_commands/templates/slash_commands/invalid_args.txt @@ -0,0 +1,10 @@ +### ⚠️ Invalid Arguments for Slash Command + +`@{{ bot_name }} /{{ command }} {{ invalid_args }}` aren't recognised arguments for **/{{ command }}**. + +**Here's how to use it correctly:** + +{{ help }} + +Need more options? Comment **`@{{ bot_name }} /help`** to see the full slash command reference. + diff --git a/daiv/quick_actions/templates/quick_actions/quick_actions_help.txt b/daiv/slash_commands/templates/slash_commands/slash_commands_help.txt similarity index 77% rename from daiv/quick_actions/templates/quick_actions/quick_actions_help.txt rename to daiv/slash_commands/templates/slash_commands/slash_commands_help.txt index 4f883642..4b9ab9c0 100644 --- a/daiv/quick_actions/templates/quick_actions/quick_actions_help.txt +++ b/daiv/slash_commands/templates/slash_commands/slash_commands_help.txt @@ -1,4 +1,4 @@ -### 🤖 {{ bot_name }} Quick-Actions +### 🤖 {{ bot_name }} Slash Commands Comment **one** of the commands below on this {{ scope }} to trigger the bot: {% for action in actions %}{{ action }} diff --git a/docker/production/app/Dockerfile b/docker/production/app/Dockerfile index e8777db6..fdfa09ad 100644 --- a/docker/production/app/Dockerfile +++ b/docker/production/app/Dockerfile @@ -89,7 +89,7 @@ WORKDIR /home/daiv RUN chmod +x entrypoint start-app start-worker start-crontask \ && python -m compileall app \ && django-admin compilemessages --ignore=.venv/**/locale \ - && mkdir -p data/tantivy_index data/media data/static data/mcp-proxy + && mkdir -p data/media data/static data/mcp-proxy HEALTHCHECK --interval=10s --start-period=30s \ CMD curl --fail http://127.0.0.1:8000/-/alive/ || exit 1 diff --git a/docs/configuration/yaml-config.md b/docs/configuration/yaml-config.md index ab15632e..af3b7564 100644 --- a/docs/configuration/yaml-config.md +++ b/docs/configuration/yaml-config.md @@ -29,7 +29,7 @@ issue_addressing: code_review: enabled: true -quick_actions: +slash_commands: enabled: true # Sandbox @@ -89,10 +89,10 @@ issue_addressing: |-----------|--------|---------|---------------------------------------------| | `enabled` | `bool` | `true` | Enable the [code review addressor feature](../features/review-addressor.md). | -### Quick Actions +### Slash Commands | Option | Type | Default | Description | |-----------|--------|---------|---------------------------------------------| -| `enabled` | `bool` | `true` | Enable [quick actions feature](../features/quick-actions.md). | +| `enabled` | `bool` | `true` | Enable [slash commands feature](../features/slash-commands.md). | !!! tip Disable features you do not need to reduce noise and speed up processing. diff --git a/docs/features/quick-actions.md b/docs/features/slash-commands.md similarity index 61% rename from docs/features/quick-actions.md rename to docs/features/slash-commands.md index b61c4447..43ccc4d8 100644 --- a/docs/features/quick-actions.md +++ b/docs/features/slash-commands.md @@ -1,16 +1,17 @@ -# ⚡ Quick Actions +# ⚡ Slash Commands -Quick Actions provide command-based interactions with DAIV directly from issues and merge/pull requests. They are useful for common tasks and information requests. +Slash commands provide command-based interactions with DAIV directly from issues and merge/pull requests. They are +useful for common tasks and information requests. --- -## Quick Actions vs. Direct Mentions +## Slash commands vs. direct mentions DAIV responds to two types of interactions: | Interaction Type | Format | Use Case | |------------------|--------|----------| -| **Quick Actions** | `@daiv /command` | Execute specific commands (get help, clone issues) | +| **Slash Commands** | `@daiv /command` | Execute specific commands (get help, clone issues) | | **Direct Mentions** | `@daiv ` | Address code review comments, ask questions, request code changes | **To address code review comments**, use a direct mention without a slash command. See [Review Addressor](review-addressor.md) for details and examples. @@ -19,11 +20,11 @@ DAIV responds to two types of interactions: ## Overview -Quick Actions are triggered by mentioning DAIV with specific commands in issue or merge/pull request comments. +Slash commands are triggered by mentioning DAIV with specific commands in issue or merge/pull request comments. -### How Quick Actions Work +### How slash commands work -**Command Format**: `@ / [arguments]` +**Command Format**: `@ / [arguments]` **Supported Scopes**: @@ -32,7 +33,7 @@ Quick Actions are triggered by mentioning DAIV with specific commands in issue o **Command Parsing**: -Quick Actions use shell-like parsing with support for: +Slash commands use shell-like parsing with support for: - **Simple commands**: `@daiv /help` - **Commands with arguments**: `@daiv /clone-to-topic backend, api` @@ -44,23 +45,23 @@ Quick Actions use shell-like parsing with support for: graph TD A["👤 User"] --> B["💬 Comments with @daiv
(e.g., '@daiv /help')"] B --> C["🔔 Comment Webhook"] - C --> D["📝 Quick Action Parser
(extracts command and args)"] - D --> E["📋 Registry Lookup
(finds matching action)"] + C --> D["📝 Slash Command Parser
(extracts command and args)"] + D --> E["📋 Registry Lookup
(finds matching command)"] - E --> F["✅ Action Found?"] - F -->|Yes| G["⚡ Execute Action"] - F -->|No| H["❌ Unknown Action Error"] + E --> F["✅ Command Found?"] + F -->|Yes| G["⚡ Execute Command"] + F -->|No| H["❌ Unknown Command Error"] G --> I["🔍 Validate Scope
(Issue vs Merge/Pull Request)"] I --> J["🛠️ Execute Specific Logic"] - J --> K["📖 Help Action
(show available commands)"] + J --> K["📖 Help Command
(show available commands)"] J --> R["📤 Clone to Topic
(clone issue to repos)"] K --> N["💬 Posts Help Message"] R --> S["📋 Creates Issues in
Matching Repositories"] - H --> Q["💬 Posts Error Message
(suggests valid actions)"] + H --> Q["💬 Posts Error Message
(suggests valid commands)"] style B fill:#e1f5fe style E fill:#fff3e0 @@ -68,22 +69,22 @@ graph TD style H fill:#ffebee ``` -### Basic Usage +### Basic usage 1. **Navigate** to any issue or merge/pull request -2. **Add a comment** mentioning DAIV with the desired action +2. **Add a comment** mentioning DAIV with the desired command 3. **Submit** the comment -4. **DAIV responds** with the action result +4. **DAIV responds** with the command result --- -## Available Quick Actions +## Available slash commands -### 🆘 Help Action +### 🆘 Help command **Command**: `/help` -**Purpose**: Displays all available Quick Actions for the current scope (issue or merge/pull request). +**Purpose**: Displays all available slash commands for the current scope (issue or merge/pull request). **Scopes**: Issues, Merge/Pull Requests @@ -92,11 +93,11 @@ graph TD @daiv /help ``` -**Response**: DAIV replies with a formatted list of all available Quick Actions and their descriptions. +**Response**: DAIV replies with a formatted list of all available slash commands and their descriptions. --- -### 📤 Clone to Topic Action +### 📤 Clone to topic command **Command**: `/clone-to-topic ` @@ -126,12 +127,12 @@ graph TD ## Troubleshooting -### Common Issues +### Common issues -**Action not recognized**: +**Command not recognized**: -- Check that the action supports the current scope (issue vs merge/pull request) -- Ensure proper spelling and case (actions are case-insensitive) +- Check that the command supports the current scope (issue vs merge/pull request) +- Ensure proper spelling and case (commands are case-insensitive) - Verify command syntax (e.g., `/help` not `/Help`) **No response from DAIV**: @@ -143,24 +144,24 @@ graph TD **Permission errors**: - Ensure DAIV has sufficient repository permissions -- Confirm the user triggering the action has appropriate access levels +- Confirm the user triggering the command has appropriate access levels -**Pipeline action issues**: +**Pipeline command issues**: - Ensure the pipeline is in "failed" status - Check that failed jobs have `script_failure` as the failure reason - Verify jobs are not marked as `allow_failure` -**Clone to topic action issues**: +**Clone to topic command issues**: - Ensure you provide at least one topic - Check that target repositories have the specified topics configured - Verify DAIV has access to the target repositories - Confirm the current repository is not the only one matching the topics -### Debug Information +### Debug information -Quick Actions log detailed information for troubleshooting: +Slash commands log detailed information for troubleshooting: - Command parsing results - Registry lookup attempts @@ -171,7 +172,7 @@ Quick Actions log detailed information for troubleshooting: ## Examples -### Getting Help +### Getting help ``` @daiv /help @@ -179,16 +180,16 @@ Quick Actions log detailed information for troubleshooting: **Response**: ``` -### 🤖 DAIV Quick-Actions +### 🤖 DAIV Slash Commands Comment one of the commands below on this issue to trigger the bot: -- `@daiv /help` - Shows the help message with the available quick actions. +- `@daiv /help` - Shows the help message with the available slash commands. - `@daiv /clone-to-topic ` - Clone this issue to all repositories matching the specified topics. ``` --- -### Cloning an Issue to Multiple Repositories +### Cloning an issue to multiple repositories ``` @daiv /clone-to-topic backend, api @@ -204,20 +205,20 @@ Cloned issue to `3` repositories: --- -## Extension and Development +## Extension and development -### Adding New Actions +### Adding new commands -1. **Create** new action class in `quick_actions/actions/` -2. **Implement** required methods `execute_action` and `actions` -3. **Decorate** with `@quick_action` specifying command and scopes +1. **Create** new command class in `slash_commands/actions/` +2. **Implement** required methods `execute_action_for_issue`, `execute_action_for_merge_request`, and `execute_for_agent` +3. **Decorate** with `@slash_command` specifying command and scopes 4. **Import** in the actions module -5. **Test** the action in development environment +5. **Test** the command in development environment -### Best Practices +### Best practices -- **Keep actions simple**: Quick Actions should execute immediately -- **Provide clear descriptions**: Help users understand what each action does +- **Keep commands simple**: Slash commands should execute immediately +- **Provide clear descriptions**: Help users understand what each command does - **Handle errors gracefully**: Post user-friendly error messages -- **Use appropriate scopes**: Only enable actions where they make sense +- **Use appropriate scopes**: Only enable commands where they make sense - **Follow naming conventions**: Use clear, descriptive command names diff --git a/docs/index.md b/docs/index.md index 38f06c69..ab190ecf 100644 --- a/docs/index.md +++ b/docs/index.md @@ -24,7 +24,7 @@ DAIV automates three key software engineering activities: ### Workflow Overview -DAIV responds to repository events with specialized workflows for each feature (Issue Addressor, Code Review Response, Quick Actions): +DAIV responds to repository events with specialized workflows for each feature (Issue Addressor, Code Review Response, Slash Commands): ```mermaid graph TD @@ -39,7 +39,7 @@ graph TD F --> J["🗂️ Clear cache"] H --> K["💬 Code Review Response
(changes or answers)"] - H --> L["⚡ Quick Action
(help, clone commands)"] + H --> L["⚡ Slash Command
(help, clone commands)"] G --> M["📤 Creates Merge/Pull Request"] K --> N["📝 Updates Code or Replies"] @@ -58,7 +58,7 @@ DAIV integrates with major Git platforms to automate your development workflow: - [:simple-gitlab: **GitLab**](https://gitlab.com) - Full feature support (GitLab.com and self-hosted) - [:simple-github: **GitHub**](https://github.com) - Full feature support (GitHub.com and GitHub Enterprise) -Both platforms support all core features including Issue Addressing, Code Review Response, and Quick Actions. +Both platforms support all core features including Issue Addressing, Code Review Response, and Slash Commands. --- diff --git a/evals/conftest.py b/evals/conftest.py index bf15b4d4..b0a5e67e 100644 --- a/evals/conftest.py +++ b/evals/conftest.py @@ -1,9 +1,10 @@ import pytest_asyncio +from codebase.base import Scope from codebase.context import set_runtime_ctx @pytest_asyncio.fixture(scope="session", loop_scope="session", autouse=True) async def runtime_ctx(): - async with set_runtime_ctx(repo_id="srtab/daiv", ref="main") as ctx: + async with set_runtime_ctx(repo_id="srtab/daiv", scope=Scope.GLOBAL, ref="main") as ctx: yield ctx diff --git a/evals/swebench.py b/evals/swebench.py index 486c9b21..05c69f5f 100644 --- a/evals/swebench.py +++ b/evals/swebench.py @@ -12,7 +12,7 @@ from automation.agent.constants import ModelName from automation.agent.graph import create_daiv_agent -from codebase.base import GitPlatform +from codebase.base import GitPlatform, Scope from codebase.context import set_runtime_ctx from codebase.utils import GitManager @@ -37,6 +37,7 @@ async def main( async with set_runtime_ctx( item["repo"], + scope=Scope.GLOBAL, ref=item["base_commit"], offline=True, git_platform=GitPlatform.SWE, diff --git a/evals/test_pr_describer.py b/evals/test_pr_describer.py index c1e37e09..d77acb1c 100644 --- a/evals/test_pr_describer.py +++ b/evals/test_pr_describer.py @@ -6,7 +6,7 @@ from automation.agent.constants import ModelName from automation.agent.pr_describer.graph import create_pr_describer_agent -from codebase.base import GitPlatform +from codebase.base import GitPlatform, Scope from codebase.context import set_runtime_ctx from .evaluators import correctness_evaluator @@ -45,7 +45,9 @@ async def test_pr_describer(inputs, reference_outputs): t.log_inputs(inputs) t.log_reference_outputs(reference_outputs) - async with set_runtime_ctx("srtab/daiv", ref="main", offline=True, git_platform=GitPlatform.GITLAB) as ctx: + async with set_runtime_ctx( + "srtab/daiv", scope=Scope.GLOBAL, ref="main", offline=True, git_platform=GitPlatform.GITLAB + ) as ctx: agent_path = Path(ctx.repo.working_dir) if "context_file_content" in inputs: (agent_path / ctx.config.context_file_name).write_text(inputs.pop("context_file_content")) diff --git a/mkdocs.yml b/mkdocs.yml index 7b8a4b31..f2a94473 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -88,7 +88,7 @@ nav: - Features: - Issue Addressor: features/issue-addressor.md - Review Addressor: features/review-addressor.md - - Quick Actions: features/quick-actions.md + - Slash Commands: features/slash-commands.md - AI Agents: - Overview: ai-agents/overview.md - Agent Skills: ai-agents/skills.md diff --git a/pyproject.toml b/pyproject.toml index 5e070e9a..c0c42e31 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -140,7 +140,7 @@ lint.isort.known-first-party = [ "chat", "codebase", "core", - "quick_actions", + "slash_commands", "daiv", ] lint.isort.section-order = [ diff --git a/tests/unit_tests/codebase/test_config.py b/tests/unit_tests/codebase/test_config.py index c4283ffd..b1c1e213 100644 --- a/tests/unit_tests/codebase/test_config.py +++ b/tests/unit_tests/codebase/test_config.py @@ -11,7 +11,7 @@ def test_get_config_from_cache(self, mock_cache): "default_branch": "main", "code_review": {"enabled": True}, "issue_addressing": {"enabled": True}, - "quick_actions": {"enabled": True}, + "slash_commands": {"enabled": True}, } mock_cache.get.return_value = cached_config @@ -20,7 +20,7 @@ def test_get_config_from_cache(self, mock_cache): assert config.default_branch == "main" assert config.code_review.enabled is True assert config.issue_addressing.enabled is True - assert config.quick_actions.enabled is True + assert config.slash_commands.enabled is True mock_cache.get.assert_called_once_with(f"{CONFIGURATION_CACHE_KEY_PREFIX}{repo_id}") @patch("codebase.repo_config.cache") @@ -34,7 +34,7 @@ def test_get_config_from_repo(self, mock_cache, mock_repo_client): enabled: true issue_addressing: enabled: true - quick_actions: + slash_commands: enabled: true """ @@ -43,7 +43,7 @@ def test_get_config_from_repo(self, mock_cache, mock_repo_client): assert config.default_branch == "main" assert config.code_review.enabled is True assert config.issue_addressing.enabled is True - assert config.quick_actions.enabled is True + assert config.slash_commands.enabled is True mock_cache.set.assert_called_once_with( f"{CONFIGURATION_CACHE_KEY_PREFIX}{repo_id}", config.model_dump(), CONFIGURATION_CACHE_TIMEOUT ) @@ -60,7 +60,7 @@ def test_get_config_with_default_values(self, mock_cache, mock_repo_client): assert config.default_branch == "main" assert config.code_review.enabled is True assert config.issue_addressing.enabled is True - assert config.quick_actions.enabled is True + assert config.slash_commands.enabled is True mock_cache.set.assert_called_once_with( f"{CONFIGURATION_CACHE_KEY_PREFIX}{repo_id}", config.model_dump(), CONFIGURATION_CACHE_TIMEOUT ) @@ -83,7 +83,7 @@ def test_get_config_with_invalid_yaml(self, mock_cache, mock_repo_client): assert config.default_branch == "main" assert config.code_review.enabled is True assert config.issue_addressing.enabled is True - assert config.quick_actions.enabled is True + assert config.slash_commands.enabled is True mock_cache.set.assert_called_once_with( f"{CONFIGURATION_CACHE_KEY_PREFIX}{repo_id}", config.model_dump(), CONFIGURATION_CACHE_TIMEOUT ) @@ -102,7 +102,7 @@ def test_get_config_with_partial_yaml(self, mock_cache, mock_repo_client): assert config.default_branch == "main" assert config.code_review.enabled is True assert config.issue_addressing.enabled is True - assert config.quick_actions.enabled is True + assert config.slash_commands.enabled is True mock_cache.set.assert_called_once_with( f"{CONFIGURATION_CACHE_KEY_PREFIX}{repo_id}", config.model_dump(), CONFIGURATION_CACHE_TIMEOUT ) diff --git a/tests/unit_tests/quick_actions/test_actions.py b/tests/unit_tests/quick_actions/test_actions.py deleted file mode 100644 index a20b6a3c..00000000 --- a/tests/unit_tests/quick_actions/test_actions.py +++ /dev/null @@ -1,117 +0,0 @@ -from unittest.mock import MagicMock, patch - -from quick_actions.actions.help import HelpQuickAction -from quick_actions.base import Scope - - -class TestHelpAction: - def setup_method(self): - """Set up test fixtures.""" - self.action = HelpQuickAction() - self.action.client = MagicMock(current_user=MagicMock(username="bot")) - - self.mock_comment = MagicMock() - self.mock_comment.id = "disc-123" - - self.mock_user = MagicMock() - self.mock_user.id = 1 - self.mock_user.username = "testuser" - - self.mock_issue = MagicMock() - self.mock_issue.id = 1 - self.mock_issue.iid = 100 - - self.mock_merge_request = MagicMock() - self.mock_merge_request.merge_request_id = 1 - - def test_help_action_has_correct_attributes(self): - """Test that HelpAction has the expected attributes set by decorator.""" - assert hasattr(HelpQuickAction, "command") - assert hasattr(HelpQuickAction, "scopes") - assert HelpQuickAction.command == "help" - assert Scope.ISSUE in HelpQuickAction.scopes - assert Scope.MERGE_REQUEST in HelpQuickAction.scopes - - @patch("quick_actions.actions.help.quick_action_registry") - async def test_execute_on_issue(self, mock_registry): - """Test executing help action on an issue.""" - # Setup mock registry with actions - mock_instance1 = MagicMock(command="help") - mock_instance1.help.return_value = "- `@bot /help` - Shows help" - mock_action1 = MagicMock(command="help") - mock_action1.return_value = mock_instance1 - - mock_instance2 = MagicMock(command="status") - mock_instance2.help.return_value = "- `@bot /status` - Shows status" - mock_action2 = MagicMock(command="status") - mock_action2.return_value = mock_instance2 - - mock_registry.get_actions.return_value = [mock_action1, mock_action2] - - # Execute the action - await self.action.execute_for_issue( - repo_id="repo123", args="", comment=self.mock_comment, issue=self.mock_issue - ) - - # Verify registry was called with correct scope - mock_registry.get_actions.assert_called_once_with(scope=Scope.ISSUE) - - # Verify issue discussion note was created - self.action.client.create_issue_comment.assert_called_once() - - @patch("quick_actions.actions.help.quick_action_registry") - async def test_execute_on_merge_request(self, mock_registry): - """Test executing help action on a merge request.""" - # Setup mock registry with actions - mock_instance = MagicMock(command="help") - mock_instance.help.return_value = "- `@bot /help` - Shows help" - mock_action = MagicMock(command="help") - mock_action.return_value = mock_instance - - mock_registry.get_actions.return_value = [mock_action] - - # Execute the action - await self.action.execute_for_merge_request( - repo_id="repo123", args="", comment=self.mock_comment, merge_request=self.mock_merge_request - ) - - # Verify registry was called with correct scope - mock_registry.get_actions.assert_called_once_with(scope=Scope.MERGE_REQUEST) - - # Verify merge request discussion note was created - self.action.client.create_merge_request_comment.assert_called_once() - - @patch("quick_actions.actions.help.quick_action_registry") - async def test_execute_with_multiple_actions(self, mock_registry): - """Test executing help action with multiple available actions.""" - # Setup mock registry with multiple actions - mock_actions = [] - action_commands = ["help", "status", "assign", "close"] - descriptions = ["Shows help", "Shows status", "Assigns issue", "Closes issue"] - - for command, desc in zip(action_commands, descriptions, strict=False): - # Create a mock instance with help() method - mock_instance = MagicMock(command=command) - mock_instance.help.return_value = f"- `@daivbot /{command}` - {desc}" - - # Create a mock class that returns the instance when called - mock_action_class = MagicMock(command=command) - mock_action_class.return_value = mock_instance - mock_actions.append(mock_action_class) - - mock_registry.get_actions.return_value = mock_actions - - # Execute the action - await self.action.execute_for_issue( - repo_id="repo123", args="", comment=self.mock_comment, issue=self.mock_issue - ) - - # Verify issue discussion note was created - self.action.client.create_issue_comment.assert_called_once() - call_args = self.action.client.create_issue_comment.call_args - - # Verify message content contains all actions - message = call_args[0][2] - for command, desc in zip(action_commands, descriptions, strict=False): - assert f"@daivbot /{command}" in message - assert desc in message diff --git a/tests/unit_tests/quick_actions/test_base.py b/tests/unit_tests/quick_actions/test_base.py deleted file mode 100644 index e659d40c..00000000 --- a/tests/unit_tests/quick_actions/test_base.py +++ /dev/null @@ -1,51 +0,0 @@ -from typing import TYPE_CHECKING -from unittest.mock import MagicMock - -from quick_actions.base import QuickAction, Scope - -if TYPE_CHECKING: - from codebase.base import Discussion, Issue, MergeRequest - - -class TestScope: - def test_scope_enum_values(self): - """Test that Scope enum has correct values.""" - assert Scope.ISSUE == "Issue" - assert Scope.MERGE_REQUEST == "Merge Request" - - def test_scope_enum_string_representation(self): - """Test string representation of Scope enum.""" - assert str(Scope.ISSUE) == "Issue" - assert str(Scope.MERGE_REQUEST) == "Merge Request" - - -class TestQuickAction: - async def test_concrete_implementation_works_for_issue(self): - """Test that a proper concrete implementation can be instantiated.""" - - class ConcreteAction(QuickAction): - actions = [MagicMock()] - - async def execute_action_for_issue(self, repo_id: str, *, args: str, comment: Discussion, issue: Issue): - return "executed" - - action = ConcreteAction() - result = await action.execute_for_issue(args="", repo_id="repo1", comment=MagicMock(), issue=MagicMock()) - assert result == "executed" - - async def test_concrete_implementation_works_for_merge_request(self): - """Test that execute method has correct signature.""" - - class TestAction(QuickAction): - actions = [MagicMock()] - - async def execute_action_for_merge_request( - self, repo_id: str, *, args: str, comment: Discussion, merge_request: MergeRequest - ): - return "executed" - - action = TestAction() - result = await action.execute_for_merge_request( - repo_id="test_repo", args="arg1 arg2", comment=MagicMock(), merge_request=MagicMock() - ) - assert result == "executed" diff --git a/tests/unit_tests/quick_actions/test_parser.py b/tests/unit_tests/quick_actions/test_parser.py deleted file mode 100644 index 048bf627..00000000 --- a/tests/unit_tests/quick_actions/test_parser.py +++ /dev/null @@ -1,163 +0,0 @@ -import pytest - -from quick_actions.parser import parse_quick_action - - -class TestParseQuickAction: - def test_parse_simple_command(self): - """Test parsing a simple bot command.""" - note_body = "@testbot /help" - result = parse_quick_action(note_body, "testbot") - - assert result is not None - assert result.command == "help" - assert result.args == [] - assert result.raw == "@testbot /help" - - def test_parse_command_with_args(self): - """Test parsing a command with arguments.""" - note_body = "@testbot /assign user1 user2" - result = parse_quick_action(note_body, "testbot") - - assert result is not None - assert result.command == "assign" - assert result.args == ["user1", "user2"] - assert result.raw == "@testbot /assign user1 user2" - - def test_parse_command_with_quoted_args(self): - """Test parsing a command with quoted arguments.""" - note_body = '@testbot /create "test issue" --label "bug fix"' - result = parse_quick_action(note_body, "testbot") - - assert result is not None - assert result.command == "create" - assert result.args == ["test issue", "--label", "bug fix"] - assert result.raw == '@testbot /create "test issue" --label "bug fix"' - - def test_parse_case_insensitive_bot_name(self): - """Test that bot name matching is case insensitive.""" - note_body = "@TestBot /help" - result = parse_quick_action(note_body, "testbot") - - assert result is not None - assert result.command == "help" - assert result.raw == "@TestBot /help" - - def test_parse_case_insensitive_command(self): - """Test that command is converted to lowercase.""" - note_body = "@testbot /HELP" - result = parse_quick_action(note_body, "testbot") - - assert result is not None - assert result.command == "help" # Should be lowercase - - def test_parse_command_in_middle_of_text(self): - """Test parsing command that appears in middle of note.""" - note_body = "Some text before\n@testbot /help\nSome text after" - result = parse_quick_action(note_body, "testbot") - - assert result is not None - assert result.command == "help" - assert result.raw == "@testbot /help" - - def test_parse_first_command_only(self): - """Test that only first command is parsed when multiple exist.""" - note_body = "@testbot /help\n@testbot /status" - result = parse_quick_action(note_body, "testbot") - - assert result is not None - assert result.command == "help" - assert result.raw == "@testbot /help" - - def test_parse_no_command_found(self): - """Test when no bot command is found.""" - note_body = "Just some regular text without commands" - result = parse_quick_action(note_body, "testbot") - - assert result is None - - def test_parse_different_bot_name(self): - """Test that commands for different bots are ignored.""" - note_body = "@otherbot /help" - result = parse_quick_action(note_body, "testbot") - - assert result is None - - def test_parse_email_address_ignored(self): - """Test that email addresses are not matched as bot commands.""" - note_body = "Contact testbot@example.com for help" - result = parse_quick_action(note_body, "testbot") - - assert result is None - - def test_parse_partial_bot_name_ignored(self): - """Test that partial bot name matches are ignored.""" - note_body = "@testbotx /help" # Extra character - result = parse_quick_action(note_body, "testbot") - - assert result is None - - def test_parse_bot_name_with_special_chars(self): - """Test parsing bot name that contains special regex characters.""" - note_body = "@test.bot /help" - result = parse_quick_action(note_body, "test.bot") - - assert result is not None - assert result.command == "help" - - def test_parse_command_with_newline_in_middle(self): - """Test that commands stop at newlines.""" - note_body = "@testbot /help arg1\nthis should not be included" - result = parse_quick_action(note_body, "testbot") - - assert result is not None - assert result.command == "help" - assert result.args == ["arg1"] - assert "this should not be included" not in result.raw - - def test_parse_empty_command(self): - """Test parsing when bot is mentioned but no command follows.""" - note_body = "@testbot " # Just whitespace after mention - result = parse_quick_action(note_body, "testbot") - - assert result is None - - def test_parse_command_with_tabs(self): - """Test parsing command with tab characters.""" - note_body = "@testbot\t\t/help\targ1" - result = parse_quick_action(note_body, "testbot") - - assert result is not None - assert result.command == "help" - assert result.args == ["arg1"] - - def test_parse_command_with_complex_quoting(self): - """Test parsing command with complex shell-style quoting.""" - note_body = "@testbot /create 'single quotes' \"double quotes\" unquoted" - result = parse_quick_action(note_body, "testbot") - - assert result is not None - assert result.command == "create" - assert result.args == ["single quotes", "double quotes", "unquoted"] - - def test_parse_command_shlex_error_handling(self): - """Test handling of shlex parsing errors (unmatched quotes).""" - note_body = '@testbot /create "unmatched quote' - result = parse_quick_action(note_body, "testbot") - - assert result is None - - def test_parse_empty_note_body(self): - """Test parsing empty note body.""" - result = parse_quick_action("", "testbot") - assert result is None - - def test_parse_none_note_body(self): - """Test parsing None note body.""" - with pytest.raises(TypeError): - parse_quick_action(None, "testbot") # type: ignore - - def test_parse_whitespace_only_note_body(self): - """Test parsing note body with only whitespace.""" - result = parse_quick_action(" \n\t ", "testbot") - assert result is None diff --git a/tests/unit_tests/quick_actions/test_registry.py b/tests/unit_tests/quick_actions/test_registry.py deleted file mode 100644 index e0eb7cb8..00000000 --- a/tests/unit_tests/quick_actions/test_registry.py +++ /dev/null @@ -1,219 +0,0 @@ -import pytest - -from quick_actions.base import QuickAction, Scope -from quick_actions.registry import QuickActionRegistry - - -class MockAction1(QuickAction): - @property - def description(self): - return "Mock action 1" - - def execute(self, repo_id, scope, note, user, issue=None, merge_request=None, args=None): - pass - - -class MockAction2(QuickAction): - @property - def description(self): - return "Mock action 2" - - def execute(self, repo_id, scope, note, user, issue=None, merge_request=None, args=None): - pass - - -class NotAnAction: - """Class that doesn't inherit from QuickAction.""" - - pass - - -class TestQuickActionRegistry: - def test_registry_initialization(self): - """Test that registry initializes with empty state.""" - registry = QuickActionRegistry() - assert registry._registry == {} - assert registry._registry_by_scope == {} - - def test_register_valid_action(self): - """Test registering a valid quick action.""" - registry = QuickActionRegistry() - scopes = [Scope.ISSUE, Scope.MERGE_REQUEST] - - registry.register(MockAction1, "test_command", scopes) - - assert "test_command" in registry._registry - assert registry._registry["test_command"] == MockAction1 - assert getattr(MockAction1, "command", None) == "test_command" - assert getattr(MockAction1, "scopes", None) == scopes - assert MockAction1 in registry._registry_by_scope[Scope.ISSUE.value] - assert MockAction1 in registry._registry_by_scope[Scope.MERGE_REQUEST.value] - - def test_register_invalid_class_raises_assertion(self): - """Test that registering non-QuickAction class raises AssertionError.""" - registry = QuickActionRegistry() - - with pytest.raises(AssertionError, match="must be a class that inherits from QuickAction"): - registry.register(NotAnAction, "invalid", [Scope.ISSUE]) # type: ignore - - def test_register_non_class_raises_assertion(self): - """Test that registering non-class raises AssertionError.""" - registry = QuickActionRegistry() - - with pytest.raises(AssertionError, match="must be a class that inherits from QuickAction"): - registry.register("not_a_class", "invalid", [Scope.ISSUE]) # type: ignore - - def test_register_duplicate_action_class_raises_assertion(self): - """Test that registering same action class twice raises AssertionError.""" - registry = QuickActionRegistry() - - registry.register(MockAction1, "first_command", [Scope.ISSUE]) - - with pytest.raises(AssertionError, match="is already registered as quick action"): - registry.register(MockAction1, "second_command", [Scope.ISSUE]) - - def test_register_duplicate_command_raises_assertion(self): - """Test that registering same command twice raises AssertionError.""" - registry = QuickActionRegistry() - - registry.register(MockAction1, "duplicate_command", [Scope.ISSUE]) - - with pytest.raises(AssertionError, match="is already registered as quick action"): - registry.register(MockAction2, "duplicate_command", [Scope.ISSUE]) - - def test_register_multiple_scopes(self): - """Test registering action with multiple scopes.""" - registry = QuickActionRegistry() - scopes = [Scope.ISSUE, Scope.MERGE_REQUEST] - - registry.register(MockAction1, "multi_scope", scopes) - - assert MockAction1 in registry._registry_by_scope[Scope.ISSUE.value] - assert MockAction1 in registry._registry_by_scope[Scope.MERGE_REQUEST.value] - - def test_register_single_scope(self): - """Test registering action with single scope.""" - registry = QuickActionRegistry() - scopes = [Scope.ISSUE] - - registry.register(MockAction1, "single_scope", scopes) - - assert MockAction1 in registry._registry_by_scope[Scope.ISSUE.value] - assert ( - Scope.MERGE_REQUEST.value not in registry._registry_by_scope - or MockAction1 not in registry._registry_by_scope[Scope.MERGE_REQUEST.value] - ) - - def test_get_actions_no_filters(self): - """Test getting all actions without filters.""" - registry = QuickActionRegistry() - - registry.register(MockAction1, "action1", [Scope.ISSUE]) - registry.register(MockAction2, "action2", [Scope.MERGE_REQUEST]) - - actions = registry.get_actions() - - assert len(actions) == 2 - assert MockAction1 in actions - assert MockAction2 in actions - - def test_get_actions_by_command_only(self): - """Test getting actions by command only.""" - registry = QuickActionRegistry() - - registry.register(MockAction1, "action1", [Scope.ISSUE]) - registry.register(MockAction2, "action2", [Scope.MERGE_REQUEST]) - - actions = registry.get_actions(command="action1") - - assert len(actions) == 1 - assert actions[0] == MockAction1 - - def test_get_actions_by_command_not_found(self): - """Test getting actions by non-existent command.""" - registry = QuickActionRegistry() - - registry.register(MockAction1, "action1", [Scope.ISSUE]) - - actions = registry.get_actions(command="nonexistent") - - assert len(actions) == 0 - - def test_get_actions_by_scope_only(self): - """Test getting actions by scope only.""" - registry = QuickActionRegistry() - - registry.register(MockAction1, "action1", [Scope.ISSUE]) - registry.register(MockAction2, "action2", [Scope.MERGE_REQUEST]) - - actions = registry.get_actions(scope=Scope.ISSUE) - - assert len(actions) == 1 - assert actions[0] == MockAction1 - - def test_get_actions_by_scope_not_found(self): - """Test getting actions by scope with no registered actions.""" - registry = QuickActionRegistry() - - actions = registry.get_actions(scope=Scope.ISSUE) - - assert len(actions) == 0 - - def test_get_actions_by_command_and_scope(self): - """Test getting actions by both command and scope.""" - registry = QuickActionRegistry() - - registry.register(MockAction1, "action1", [Scope.ISSUE, Scope.MERGE_REQUEST]) - registry.register(MockAction2, "action2", [Scope.ISSUE]) - - actions = registry.get_actions(command="action1", scope=Scope.ISSUE) - - assert len(actions) == 1 - assert actions[0] == MockAction1 - - def test_get_actions_by_command_and_scope_not_found(self): - """Test getting actions by command and scope with no matches.""" - registry = QuickActionRegistry() - - registry.register(MockAction1, "action1", [Scope.ISSUE]) - - actions = registry.get_actions(command="action1", scope=Scope.MERGE_REQUEST) - - assert len(actions) == 0 - - def test_get_actions_multiple_actions_same_scope(self): - """Test getting multiple actions for same scope.""" - registry = QuickActionRegistry() - - registry.register(MockAction1, "action1", [Scope.ISSUE]) - registry.register(MockAction2, "action2", [Scope.ISSUE]) - - actions = registry.get_actions(scope=Scope.ISSUE) - - assert len(actions) == 2 - assert MockAction1 in actions - assert MockAction2 in actions - - def test_action_attributes_set_correctly(self): - """Test that action class attributes are set correctly during registration.""" - registry = QuickActionRegistry() - original_command = getattr(MockAction1, "command", None) - original_scopes = getattr(MockAction1, "scopes", None) - - try: - scopes = [Scope.ISSUE, Scope.MERGE_REQUEST] - registry.register(MockAction1, "test_attributes", scopes) - - assert hasattr(MockAction1, "command") and MockAction1.command == "test_attributes" # type: ignore - assert hasattr(MockAction1, "scopes") and MockAction1.scopes == scopes # type: ignore - finally: - # Clean up - restore original attributes if they existed - if original_command is not None: - MockAction1.command = original_command - elif hasattr(MockAction1, "command"): - delattr(MockAction1, "command") - - if original_scopes is not None: - MockAction1.scopes = original_scopes - elif hasattr(MockAction1, "scopes"): - delattr(MockAction1, "scopes") diff --git a/tests/unit_tests/quick_actions/test_tasks.py b/tests/unit_tests/quick_actions/test_tasks.py deleted file mode 100644 index 224dfd05..00000000 --- a/tests/unit_tests/quick_actions/test_tasks.py +++ /dev/null @@ -1,265 +0,0 @@ -from unittest.mock import AsyncMock, MagicMock, patch - -from codebase.base import Discussion, Issue, MergeRequest, Note, NoteableType, User -from quick_actions.base import Scope -from quick_actions.tasks import execute_issue_task, execute_merge_request_task - - -class TestExecuteQuickActionTask: - def setup_method(self): - """Set up test fixtures.""" - self.user = User(id=1, username="testuser", name="Test User") - self.note = Note( - id=1, body="@bot /help", author=self.user, noteable_type=NoteableType.ISSUE, system=False, resolvable=True - ) - self.discussion = Discussion(id="disc-123", notes=[self.note]) - self.issue = Issue( - id=1, iid=100, title="Test Issue", description="Test Issue Description", state="open", author=self.user - ) - self.merge_request = MergeRequest( - repo_id="repo123", - merge_request_id=200, - title="Test MR", - description="Test MR Description", - source_branch="source_branch", - target_branch="target_branch", - author=self.user, - ) - - @patch("quick_actions.tasks.quick_action_registry") - async def test_execute_action_success_issue(self, mock_registry, mock_repo_client): - """Test successful execution of quick action on issue.""" - # Setup mock action - mock_action_class = MagicMock() - mock_action_instance = MagicMock() - mock_action_instance.execute_for_issue = AsyncMock() - mock_action_class.return_value = mock_action_instance - mock_action_class.can_reply = True - mock_registry.get_actions.return_value = [mock_action_class] - - # Setup mock repo client - mock_repo_client.get_issue_comment.return_value = self.discussion - mock_repo_client.get_issue.return_value = self.issue - - # Execute task with string action args - await execute_issue_task.aenqueue( - repo_id="repo123", - action_command="help", - action_args="arg1 arg2", - comment_id=self.discussion.id, - issue_id=self.issue.iid, - ) - - # Verify registry was called correctly - mock_registry.get_actions.assert_called_once_with(command="help", scope=Scope.ISSUE) - - # Verify RepoClient calls - mock_repo_client.get_issue_comment.assert_called_once_with("repo123", self.issue.iid, self.discussion.id) - mock_repo_client.get_issue.assert_called_once_with("repo123", self.issue.iid) - - # Verify action was instantiated - mock_action_class.assert_called_once() - - # Verify the execute method was called with correct arguments - mock_action_instance.execute_for_issue.assert_called_once_with( - repo_id="repo123", comment=self.discussion, issue=self.issue, args="arg1 arg2" - ) - - @patch("quick_actions.tasks.quick_action_registry") - async def test_execute_action_success_merge_request(self, mock_registry, mock_repo_client): - """Test successful execution of quick action on merge request.""" - # Setup mock action - mock_action_class = MagicMock() - mock_action_instance = MagicMock() - mock_action_instance.execute_for_merge_request = AsyncMock() - mock_action_class.return_value = mock_action_instance - mock_action_class.can_reply = True - mock_registry.get_actions.return_value = [mock_action_class] - - # Setup mock repo client - mock_repo_client.get_merge_request_comment.return_value = self.discussion - mock_repo_client.get_merge_request.return_value = self.merge_request - - # Execute task - await execute_merge_request_task.aenqueue( - repo_id="repo123", - action_command="help", - action_args="", - comment_id=self.discussion.id, - merge_request_id=self.merge_request.merge_request_id, - ) - - # Verify registry was called correctly - mock_registry.get_actions.assert_called_once_with(command="help", scope=Scope.MERGE_REQUEST) - - # Verify RepoClient calls - mock_repo_client.get_merge_request_comment.assert_called_once_with( - "repo123", self.merge_request.merge_request_id, "disc-123" - ) - mock_repo_client.get_merge_request.assert_called_once_with("repo123", self.merge_request.merge_request_id) - - # Verify action was executed - mock_action_instance.execute_for_merge_request.assert_called_once_with( - repo_id="repo123", args="", comment=self.discussion, merge_request=self.merge_request - ) - - @patch("quick_actions.tasks.quick_action_registry") - async def test_action_not_found(self, mock_registry): - """Test when quick action is not found in registry.""" - mock_registry.get_actions.return_value = [] - - # Execute task - await execute_issue_task.aenqueue( - repo_id="repo123", - action_command="nonexistent", - action_args="", - comment_id=self.discussion.id, - issue_id=self.issue.iid, - ) - - # Verify registry was called - mock_registry.get_actions.assert_called_once_with(command="nonexistent", scope=Scope.ISSUE) - - @patch("quick_actions.tasks.quick_action_registry") - async def test_multiple_actions_found(self, mock_registry): - """Test when multiple actions are found for same command/scope.""" - mock_action_class1 = MagicMock() - mock_action_class1.command = "duplicate" - mock_action_class2 = MagicMock() - mock_action_class2.command = "duplicate" - - mock_registry.get_actions.return_value = [mock_action_class1, mock_action_class2] - - # Execute task - await execute_issue_task.aenqueue( - repo_id="repo123", - action_command="duplicate", - action_args="", - comment_id=self.discussion.id, - issue_id=self.issue.iid, - ) - - # Verify registry was called - mock_registry.get_actions.assert_called_once_with(command="duplicate", scope=Scope.ISSUE) - - @patch("quick_actions.tasks.quick_action_registry") - async def test_action_execution_exception_issue(self, mock_registry, mock_repo_client): - """Test handling of exception during action execution on issue.""" - # Setup mock action that raises exception - mock_action_class = MagicMock() - mock_action_instance = MagicMock() - mock_action_instance.execute_for_issue = AsyncMock(side_effect=Exception("Action failed")) - mock_action_class.return_value = mock_action_instance - mock_action_class.can_reply = True - mock_registry.get_actions.return_value = [mock_action_class] - - # Setup mock repo client - mock_repo_client.current_user = self.user - mock_repo_client.get_issue_comment.return_value = self.discussion - mock_repo_client.get_issue.return_value = self.issue - - # Execute task - await execute_issue_task.aenqueue( - repo_id="repo123", - action_command="failing_action", - action_args="", - comment_id=self.discussion.id, - issue_id=self.issue.iid, - ) - - # Verify error message is posted to issue - mock_repo_client.create_issue_comment.assert_called_once() - call_args = mock_repo_client.create_issue_comment.call_args - assert call_args[0][0] == "repo123" - assert call_args[0][1] == self.issue.iid - assert "failing_action" in call_args[0][2] - assert "reply_to_id" not in call_args[1] # to make sure we don't reply to the comment as thread - - @patch("quick_actions.tasks.quick_action_registry") - async def test_action_execution_exception_merge_request(self, mock_registry, mock_repo_client): - """Test handling of exception during action execution on merge request.""" - # Setup mock action that raises exception - mock_action_class = MagicMock() - mock_action_instance = MagicMock() - mock_action_instance.execute_for_merge_request = AsyncMock(side_effect=Exception("Action failed")) - mock_action_class.return_value = mock_action_instance - mock_action_class.can_reply = True - mock_registry.get_actions.return_value = [mock_action_class] - - # Setup mock repo client - mock_repo_client.current_user = self.user - mock_repo_client.get_merge_request_comment.return_value = self.discussion - mock_repo_client.get_merge_request.return_value = self.merge_request - - # Execute task - await execute_merge_request_task.aenqueue( - repo_id="repo123", - action_command="failing_action", - action_args="", - comment_id=self.discussion.id, - merge_request_id=self.merge_request.merge_request_id, - ) - - # Verify error message is posted to merge request - mock_repo_client.create_merge_request_comment.assert_called_once() - call_args = mock_repo_client.create_merge_request_comment.call_args - assert call_args[0][0] == "repo123" - assert call_args[0][1] == self.merge_request.merge_request_id - assert "failing_action" in call_args[0][2] - assert "reply_to_id" not in call_args[1] # to make sure we don't reply to the comment as thread - - @patch("quick_actions.tasks.quick_action_registry") - async def test_scope_conversion(self, mock_registry, mock_repo_client): - """Test that string scope is converted to Scope enum.""" - mock_action_class = MagicMock() - mock_action_instance = MagicMock() - mock_action_instance.execute_for_merge_request = AsyncMock() - mock_action_class.return_value = mock_action_instance - mock_action_class.can_reply = True - mock_registry.get_actions.return_value = [mock_action_class] - - # Setup mock repo client - mock_repo_client.get_merge_request_comment.return_value = self.discussion - mock_repo_client.get_merge_request.return_value = self.merge_request - - # Execute task with string scope - await execute_merge_request_task.aenqueue( - repo_id="repo123", - action_command="help", - action_args="", - comment_id=self.discussion.id, - merge_request_id=self.merge_request.merge_request_id, - ) - - # Verify scope was converted to enum in both registry call and action execution - mock_registry.get_actions.assert_called_once_with(command="help", scope=Scope.MERGE_REQUEST) - - mock_action_instance.execute_for_merge_request.assert_called_once() - - @patch("quick_actions.tasks.quick_action_registry") - async def test_execute_with_empty_action_args(self, mock_registry, mock_repo_client): - """Test execution with empty action_args string.""" - mock_action_class = MagicMock() - mock_action_instance = MagicMock() - mock_action_instance.execute_for_issue = AsyncMock() - mock_action_class.return_value = mock_action_instance - mock_action_class.can_reply = True - mock_registry.get_actions.return_value = [mock_action_class] - - # Setup mock repo client - mock_repo_client.get_issue_comment.return_value = self.discussion - mock_repo_client.get_issue.return_value = self.issue - - # Execute task with empty action args - await execute_issue_task.aenqueue( - repo_id="repo123", - action_command="help", - action_args="", - comment_id=self.discussion.id, - issue_id=self.issue.iid, - ) - - # Verify action was executed with empty string - mock_action_instance.execute_for_issue.assert_called_once_with( - repo_id="repo123", comment=self.discussion, issue=self.issue, args="" - ) diff --git a/daiv/quick_actions/__init__.py b/tests/unit_tests/slash_commands/__init__.py similarity index 100% rename from daiv/quick_actions/__init__.py rename to tests/unit_tests/slash_commands/__init__.py diff --git a/tests/unit_tests/slash_commands/actions/__init__.py b/tests/unit_tests/slash_commands/actions/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit_tests/slash_commands/actions/test_help.py b/tests/unit_tests/slash_commands/actions/test_help.py new file mode 100644 index 00000000..750e8123 --- /dev/null +++ b/tests/unit_tests/slash_commands/actions/test_help.py @@ -0,0 +1,32 @@ +from unittest.mock import MagicMock, Mock, patch + +from pytest import fixture + +from codebase.base import Scope +from slash_commands.actions.help import HelpSlashCommand + + +@fixture +def help_slash_command() -> HelpSlashCommand: + """Set up test fixtures.""" + command = HelpSlashCommand() + command.client = MagicMock(current_user=MagicMock(username="bot")) + return command + + +def test_help_command_has_correct_attributes(): + """Test that HelpSlashCommand has the expected attributes set by decorator.""" + assert hasattr(HelpSlashCommand, "command") + assert hasattr(HelpSlashCommand, "scopes") + assert HelpSlashCommand.command == "help" + assert Scope.ISSUE in HelpSlashCommand.scopes + assert Scope.MERGE_REQUEST in HelpSlashCommand.scopes + + +@patch("slash_commands.actions.help.slash_command_registry.get_commands", new=Mock(return_value=[])) +async def test_help_command_returns_correct_message(help_slash_command: HelpSlashCommand): + """Test that HelpSlashCommand returns the correct message.""" + message = await help_slash_command.execute_for_agent( + args="", scope=Scope.ISSUE, repo_id="repo1", bot_username="bot" + ) + assert message == "No slash commands available." diff --git a/tests/unit_tests/quick_actions/test_decorator.py b/tests/unit_tests/slash_commands/test_decorator.py similarity index 50% rename from tests/unit_tests/quick_actions/test_decorator.py rename to tests/unit_tests/slash_commands/test_decorator.py index acc6904d..3c30dd84 100644 --- a/tests/unit_tests/quick_actions/test_decorator.py +++ b/tests/unit_tests/slash_commands/test_decorator.py @@ -1,15 +1,16 @@ from typing import TYPE_CHECKING from unittest.mock import MagicMock, patch -from quick_actions.base import QuickAction, Scope -from quick_actions.decorator import quick_action +from codebase.base import Scope +from slash_commands.base import SlashCommand +from slash_commands.decorator import slash_command if TYPE_CHECKING: from codebase.base import Discussion, Issue, MergeRequest, Note -class TestQuickActionDecorator: - class TestAction(QuickAction): +class TestSlashCommandDecorator: + class TestCommand(SlashCommand): actions = [MagicMock()] async def execute_action( @@ -26,33 +27,33 @@ async def execute_action( ) -> None: pass - def test_decorator_with_valid_action(self): - """Test that decorator properly registers a valid quick action.""" - with patch("quick_actions.decorator.quick_action_registry") as mock_registry: - quick_action(command="test_action", scopes=[Scope.ISSUE])(self.TestAction) + def test_decorator_with_valid_command(self): + """Test that decorator properly registers a valid slash command.""" + with patch("slash_commands.decorator.slash_command_registry") as mock_registry: + slash_command(command="test_command", scopes=[Scope.ISSUE])(self.TestCommand) # Verify the decorator called register with correct parameters - mock_registry.register.assert_called_once_with(self.TestAction, "test_action", [Scope.ISSUE]) + mock_registry.register.assert_called_once_with(self.TestCommand, "test_command", [Scope.ISSUE]) # Verify the class is returned unchanged - assert self.TestAction.__name__ == "TestAction" + assert self.TestCommand.__name__ == "TestCommand" def test_decorator_with_multiple_scopes(self): """Test that decorator works with multiple scopes.""" - with patch("quick_actions.decorator.quick_action_registry") as mock_registry: + with patch("slash_commands.decorator.slash_command_registry") as mock_registry: scopes = [Scope.ISSUE, Scope.MERGE_REQUEST] - quick_action(command="multi_scope_action", scopes=scopes)(self.TestAction) + slash_command(command="multi_scope_command", scopes=scopes)(self.TestCommand) - mock_registry.register.assert_called_once_with(self.TestAction, "multi_scope_action", scopes) + mock_registry.register.assert_called_once_with(self.TestCommand, "multi_scope_command", scopes) def test_decorator_can_be_applied_to_multiple_classes(self): """Test that decorator can be applied to multiple different classes.""" - with patch("quick_actions.decorator.quick_action_registry") as mock_registry: - quick_action(command="action1", scopes=[Scope.ISSUE])(self.TestAction) + with patch("slash_commands.decorator.slash_command_registry") as mock_registry: + slash_command(command="command1", scopes=[Scope.ISSUE])(self.TestCommand) - @quick_action(command="action2", scopes=[Scope.MERGE_REQUEST]) - class Action2(self.TestAction): + @slash_command(command="command2", scopes=[Scope.MERGE_REQUEST]) + class Command2(self.TestCommand): pass # Verify both registrations occurred @@ -60,23 +61,23 @@ class Action2(self.TestAction): # Check the specific calls calls = mock_registry.register.call_args_list - assert calls[0][0] == (self.TestAction, "action1", [Scope.ISSUE]) - assert calls[1][0] == (Action2, "action2", [Scope.MERGE_REQUEST]) + assert calls[0][0] == (self.TestCommand, "command1", [Scope.ISSUE]) + assert calls[1][0] == (Command2, "command2", [Scope.MERGE_REQUEST]) def test_decorator_with_inheritance(self): """Test that decorator works with class inheritance.""" - with patch("quick_actions.decorator.quick_action_registry") as mock_registry: + with patch("slash_commands.decorator.slash_command_registry") as mock_registry: - class BaseAction(self.TestAction): + class BaseCommand(self.TestCommand): def shared_method(self): return "shared" - @quick_action(command="inherited_action", scopes=[Scope.ISSUE]) - class InheritedAction(BaseAction): + @slash_command(command="inherited_command", scopes=[Scope.ISSUE]) + class InheritedCommand(BaseCommand): pass - mock_registry.register.assert_called_once_with(InheritedAction, "inherited_action", [Scope.ISSUE]) + mock_registry.register.assert_called_once_with(InheritedCommand, "inherited_command", [Scope.ISSUE]) # Verify inheritance still works - action = InheritedAction() - assert action.shared_method() == "shared" + command = InheritedCommand() + assert command.shared_method() == "shared" diff --git a/tests/unit_tests/slash_commands/test_parser.py b/tests/unit_tests/slash_commands/test_parser.py new file mode 100644 index 00000000..af30ed67 --- /dev/null +++ b/tests/unit_tests/slash_commands/test_parser.py @@ -0,0 +1,309 @@ +import pytest + +from slash_commands.parser import parse_agent_slash_command, parse_slash_command + + +class TestParseSlashCommand: + def test_parse_simple_command(self): + """Test parsing a simple bot command.""" + note_body = "@testbot /help" + result = parse_slash_command(note_body, "testbot") + + assert result is not None + assert result.command == "help" + assert result.args == [] + assert result.raw == "@testbot /help" + + def test_parse_command_with_args(self): + """Test parsing a command with arguments.""" + note_body = "@testbot /assign user1 user2" + result = parse_slash_command(note_body, "testbot") + + assert result is not None + assert result.command == "assign" + assert result.args == ["user1", "user2"] + assert result.raw == "@testbot /assign user1 user2" + + def test_parse_command_with_quoted_args(self): + """Test parsing a command with quoted arguments.""" + note_body = '@testbot /create "test issue" --label "bug fix"' + result = parse_slash_command(note_body, "testbot") + + assert result is not None + assert result.command == "create" + assert result.args == ["test issue", "--label", "bug fix"] + assert result.raw == '@testbot /create "test issue" --label "bug fix"' + + def test_parse_case_insensitive_bot_name(self): + """Test that bot name matching is case insensitive.""" + note_body = "@TestBot /help" + result = parse_slash_command(note_body, "testbot") + + assert result is not None + assert result.command == "help" + assert result.raw == "@TestBot /help" + + def test_parse_case_insensitive_command(self): + """Test that command is converted to lowercase.""" + note_body = "@testbot /HELP" + result = parse_slash_command(note_body, "testbot") + + assert result is not None + assert result.command == "help" # Should be lowercase + + def test_parse_command_in_middle_of_text(self): + """Test parsing command that appears in middle of note.""" + note_body = "Some text before\n@testbot /help\nSome text after" + result = parse_slash_command(note_body, "testbot") + + assert result is not None + assert result.command == "help" + assert result.raw == "@testbot /help" + + def test_parse_first_command_only(self): + """Test that only first command is parsed when multiple exist.""" + note_body = "@testbot /help\n@testbot /status" + result = parse_slash_command(note_body, "testbot") + + assert result is not None + assert result.command == "help" + assert result.raw == "@testbot /help" + + def test_parse_no_command_found(self): + """Test when no bot command is found.""" + note_body = "Just some regular text without commands" + result = parse_slash_command(note_body, "testbot") + + assert result is None + + def test_parse_different_bot_name(self): + """Test that commands for different bots are ignored.""" + note_body = "@otherbot /help" + result = parse_slash_command(note_body, "testbot") + + assert result is None + + def test_parse_email_address_ignored(self): + """Test that email addresses are not matched as bot commands.""" + note_body = "Contact testbot@example.com for help" + result = parse_slash_command(note_body, "testbot") + + assert result is None + + def test_parse_partial_bot_name_ignored(self): + """Test that partial bot name matches are ignored.""" + note_body = "@testbotx /help" # Extra character + result = parse_slash_command(note_body, "testbot") + + assert result is None + + def test_parse_bot_name_with_special_chars(self): + """Test parsing bot name that contains special regex characters.""" + note_body = "@test.bot /help" + result = parse_slash_command(note_body, "test.bot") + + assert result is not None + assert result.command == "help" + + def test_parse_command_with_newline_in_middle(self): + """Test that commands stop at newlines.""" + note_body = "@testbot /help arg1\nthis should not be included" + result = parse_slash_command(note_body, "testbot") + + assert result is not None + assert result.command == "help" + assert result.args == ["arg1"] + assert "this should not be included" not in result.raw + + def test_parse_empty_command(self): + """Test parsing when bot is mentioned but no command follows.""" + note_body = "@testbot " # Just whitespace after mention + result = parse_slash_command(note_body, "testbot") + + assert result is None + + def test_parse_command_with_tabs(self): + """Test parsing command with tab characters.""" + note_body = "@testbot\t\t/help\targ1" + result = parse_slash_command(note_body, "testbot") + + assert result is not None + assert result.command == "help" + assert result.args == ["arg1"] + + def test_parse_command_with_complex_quoting(self): + """Test parsing command with complex shell-style quoting.""" + note_body = "@testbot /create 'single quotes' \"double quotes\" unquoted" + result = parse_slash_command(note_body, "testbot") + + assert result is not None + assert result.command == "create" + assert result.args == ["single quotes", "double quotes", "unquoted"] + + def test_parse_command_shlex_error_handling(self): + """Test handling of shlex parsing errors (unmatched quotes).""" + note_body = '@testbot /create "unmatched quote' + result = parse_slash_command(note_body, "testbot") + + assert result is None + + def test_parse_empty_note_body(self): + """Test parsing empty note body.""" + result = parse_slash_command("", "testbot") + assert result is None + + def test_parse_none_note_body(self): + """Test parsing None note body.""" + with pytest.raises(TypeError): + parse_slash_command(None, "testbot") # type: ignore + + def test_parse_whitespace_only_note_body(self): + """Test parsing note body with only whitespace.""" + result = parse_slash_command(" \n\t ", "testbot") + assert result is None + + +class TestParseAgentSlashCommand: + """Test suite for parse_agent_slash_command that supports both mention and bare slash commands.""" + + def test_parse_mention_format(self): + """Test parsing mention-based format (@bot /command).""" + text = "@testbot /help" + result = parse_agent_slash_command(text, "testbot") + + assert result is not None + assert result.command == "help" + assert result.args == [] + assert result.raw == "@testbot /help" + + def test_parse_mention_format_with_args(self): + """Test parsing mention-based format with arguments.""" + text = "@testbot /clone-to-topic backend, api" + result = parse_agent_slash_command(text, "testbot") + + assert result is not None + assert result.command == "clone-to-topic" + assert result.args == ["backend,", "api"] + assert result.raw == "@testbot /clone-to-topic backend, api" + + def test_parse_bare_slash_command(self): + """Test parsing bare slash command (/command).""" + text = "/help" + result = parse_agent_slash_command(text, "testbot") + + assert result is not None + assert result.command == "help" + assert result.args == [] + assert result.raw == "/help" + + def test_parse_bare_slash_command_with_args(self): + """Test parsing bare slash command with arguments.""" + text = "/review please check the security aspects" + result = parse_agent_slash_command(text, "testbot") + + assert result is not None + assert result.command == "review" + assert result.args == ["please", "check", "the", "security", "aspects"] + assert result.raw == "/review please check the security aspects" + + def test_parse_bare_slash_command_with_leading_whitespace(self): + """Test parsing bare slash command with leading whitespace.""" + text = " /help" + result = parse_agent_slash_command(text, "testbot") + + assert result is not None + assert result.command == "help" + assert result.args == [] + assert "/help" in result.raw + + def test_parse_bare_slash_command_in_multiline(self): + """Test parsing bare slash command in multiline text.""" + text = "Some text before\n/help\nSome text after" + result = parse_agent_slash_command(text, "testbot") + + assert result is not None + assert result.command == "help" + assert result.args == [] + + def test_parse_prioritizes_mention_over_bare(self): + """Test that mention format is prioritized over bare format.""" + text = "/bare-command\n@testbot /mention-command" + result = parse_agent_slash_command(text, "testbot") + + # Should find the mention format first + assert result is not None + assert result.command == "mention-command" + + def test_parse_bare_slash_command_with_quoted_args(self): + """Test parsing bare slash command with quoted arguments.""" + text = '/security-audit "check authentication"' + result = parse_agent_slash_command(text, "testbot") + + assert result is not None + assert result.command == "security-audit" + assert result.args == ["check authentication"] + + def test_parse_no_command_found(self): + """Test when no command is found.""" + text = "Just regular text without any commands" + result = parse_agent_slash_command(text, "testbot") + + assert result is None + + def test_parse_bare_slash_at_start_of_line(self): + """Test parsing bare slash command at the start of a line.""" + text = "/help me understand" + result = parse_agent_slash_command(text, "testbot") + + assert result is not None + assert result.command == "help" + assert result.args == ["me", "understand"] + + def test_parse_bare_slash_not_mid_word(self): + """Test that slash in middle of word is not detected.""" + text = "http://example.com/help" + result = parse_agent_slash_command(text, "testbot") + + assert result is None + + def test_parse_bare_slash_command_case_insensitive(self): + """Test that bare slash commands are converted to lowercase.""" + text = "/HELP" + result = parse_agent_slash_command(text, "testbot") + + assert result is not None + assert result.command == "help" + + def test_parse_empty_text(self): + """Test parsing empty text.""" + result = parse_agent_slash_command("", "testbot") + assert result is None + + def test_parse_whitespace_only(self): + """Test parsing whitespace-only text.""" + result = parse_agent_slash_command(" \n\t ", "testbot") + assert result is None + + def test_parse_bare_slash_shlex_error(self): + """Test handling of shlex parsing errors in bare format.""" + text = '/command "unmatched quote' + result = parse_agent_slash_command(text, "testbot") + + assert result is None + + def test_parse_bare_slash_only(self): + """Test parsing when only slash is present.""" + text = "/" + result = parse_agent_slash_command(text, "testbot") + + # Should return None as there's no command after the slash + assert result is None + + def test_parse_bare_slash_with_tabs(self): + """Test parsing bare slash command with tabs.""" + text = "/help\targ1\targ2" + result = parse_agent_slash_command(text, "testbot") + + assert result is not None + assert result.command == "help" + assert result.args == ["arg1", "arg2"] diff --git a/tests/unit_tests/slash_commands/test_registry.py b/tests/unit_tests/slash_commands/test_registry.py new file mode 100644 index 00000000..55af66f0 --- /dev/null +++ b/tests/unit_tests/slash_commands/test_registry.py @@ -0,0 +1,220 @@ +import pytest + +from codebase.base import Scope +from slash_commands.base import SlashCommand +from slash_commands.registry import SlashCommandRegistry + + +class MockCommand1(SlashCommand): + @property + def description(self): + return "Mock command 1" + + def execute(self, repo_id, scope, note, user, issue=None, merge_request=None, args=None): + pass + + +class MockCommand2(SlashCommand): + @property + def description(self): + return "Mock command 2" + + def execute(self, repo_id, scope, note, user, issue=None, merge_request=None, args=None): + pass + + +class NotACommand: + """Class that doesn't inherit from SlashCommand.""" + + pass + + +class TestSlashCommandRegistry: + def test_registry_initialization(self): + """Test that registry initializes with empty state.""" + registry = SlashCommandRegistry() + assert registry._registry == {} + assert registry._registry_by_scope == {} + + def test_register_valid_command(self): + """Test registering a valid slash command.""" + registry = SlashCommandRegistry() + scopes = [Scope.ISSUE, Scope.MERGE_REQUEST] + + registry.register(MockCommand1, "test_command", scopes) + + assert "test_command" in registry._registry + assert registry._registry["test_command"] == MockCommand1 + assert getattr(MockCommand1, "command", None) == "test_command" + assert getattr(MockCommand1, "scopes", None) == scopes + assert MockCommand1 in registry._registry_by_scope[Scope.ISSUE.value] + assert MockCommand1 in registry._registry_by_scope[Scope.MERGE_REQUEST.value] + + def test_register_invalid_class_raises_assertion(self): + """Test that registering non-SlashCommand class raises AssertionError.""" + registry = SlashCommandRegistry() + + with pytest.raises(AssertionError, match="must be a class that inherits from SlashCommand"): + registry.register(NotACommand, "invalid", [Scope.ISSUE]) # type: ignore + + def test_register_non_class_raises_assertion(self): + """Test that registering non-class raises AssertionError.""" + registry = SlashCommandRegistry() + + with pytest.raises(AssertionError, match="must be a class that inherits from SlashCommand"): + registry.register("not_a_class", "invalid", [Scope.ISSUE]) # type: ignore + + def test_register_duplicate_command_class_raises_assertion(self): + """Test that registering same command class twice raises AssertionError.""" + registry = SlashCommandRegistry() + + registry.register(MockCommand1, "first_command", [Scope.ISSUE]) + + with pytest.raises(AssertionError, match="is already registered"): + registry.register(MockCommand1, "second_command", [Scope.ISSUE]) + + def test_register_duplicate_command_raises_assertion(self): + """Test that registering same command twice raises AssertionError.""" + registry = SlashCommandRegistry() + + registry.register(MockCommand1, "duplicate_command", [Scope.ISSUE]) + + with pytest.raises(AssertionError, match="is already registered"): + registry.register(MockCommand2, "duplicate_command", [Scope.ISSUE]) + + def test_register_multiple_scopes(self): + """Test registering command with multiple scopes.""" + registry = SlashCommandRegistry() + scopes = [Scope.ISSUE, Scope.MERGE_REQUEST] + + registry.register(MockCommand1, "multi_scope", scopes) + + assert MockCommand1 in registry._registry_by_scope[Scope.ISSUE.value] + assert MockCommand1 in registry._registry_by_scope[Scope.MERGE_REQUEST.value] + + def test_register_single_scope(self): + """Test registering command with single scope.""" + registry = SlashCommandRegistry() + scopes = [Scope.ISSUE] + + registry.register(MockCommand1, "single_scope", scopes) + + assert MockCommand1 in registry._registry_by_scope[Scope.ISSUE.value] + assert ( + Scope.MERGE_REQUEST.value not in registry._registry_by_scope + or MockCommand1 not in registry._registry_by_scope[Scope.MERGE_REQUEST.value] + ) + + def test_get_commands_no_filters(self): + """Test getting all commands without filters.""" + registry = SlashCommandRegistry() + + registry.register(MockCommand1, "command1", [Scope.ISSUE]) + registry.register(MockCommand2, "command2", [Scope.MERGE_REQUEST]) + + commands = registry.get_commands() + + assert len(commands) == 2 + assert MockCommand1 in commands + assert MockCommand2 in commands + + def test_get_commands_by_command_only(self): + """Test getting commands by command only.""" + registry = SlashCommandRegistry() + + registry.register(MockCommand1, "command1", [Scope.ISSUE]) + registry.register(MockCommand2, "command2", [Scope.MERGE_REQUEST]) + + commands = registry.get_commands(command="command1") + + assert len(commands) == 1 + assert commands[0] == MockCommand1 + + def test_get_commands_by_command_not_found(self): + """Test getting commands by non-existent command.""" + registry = SlashCommandRegistry() + + registry.register(MockCommand1, "command1", [Scope.ISSUE]) + + commands = registry.get_commands(command="nonexistent") + + assert len(commands) == 0 + + def test_get_commands_by_scope_only(self): + """Test getting commands by scope only.""" + registry = SlashCommandRegistry() + + registry.register(MockCommand1, "command1", [Scope.ISSUE]) + registry.register(MockCommand2, "command2", [Scope.MERGE_REQUEST]) + + commands = registry.get_commands(scope=Scope.ISSUE) + + assert len(commands) == 1 + assert commands[0] == MockCommand1 + + def test_get_commands_by_scope_not_found(self): + """Test getting commands by scope with no registered commands.""" + registry = SlashCommandRegistry() + + commands = registry.get_commands(scope=Scope.ISSUE) + + assert len(commands) == 0 + + def test_get_commands_by_command_and_scope(self): + """Test getting commands by both command and scope.""" + registry = SlashCommandRegistry() + + registry.register(MockCommand1, "command1", [Scope.ISSUE, Scope.MERGE_REQUEST]) + registry.register(MockCommand2, "command2", [Scope.ISSUE]) + + commands = registry.get_commands(command="command1", scope=Scope.ISSUE) + + assert len(commands) == 1 + assert commands[0] == MockCommand1 + + def test_get_commands_by_command_and_scope_not_found(self): + """Test getting commands by command and scope with no matches.""" + registry = SlashCommandRegistry() + + registry.register(MockCommand1, "command1", [Scope.ISSUE]) + + commands = registry.get_commands(command="command1", scope=Scope.MERGE_REQUEST) + + assert len(commands) == 0 + + def test_get_commands_multiple_commands_same_scope(self): + """Test getting multiple commands for same scope.""" + registry = SlashCommandRegistry() + + registry.register(MockCommand1, "command1", [Scope.ISSUE]) + registry.register(MockCommand2, "command2", [Scope.ISSUE]) + + commands = registry.get_commands(scope=Scope.ISSUE) + + assert len(commands) == 2 + assert MockCommand1 in commands + assert MockCommand2 in commands + + def test_command_attributes_set_correctly(self): + """Test that command class attributes are set correctly during registration.""" + registry = SlashCommandRegistry() + original_command = getattr(MockCommand1, "command", None) + original_scopes = getattr(MockCommand1, "scopes", None) + + try: + scopes = [Scope.ISSUE, Scope.MERGE_REQUEST] + registry.register(MockCommand1, "test_attributes", scopes) + + assert hasattr(MockCommand1, "command") and MockCommand1.command == "test_attributes" # type: ignore + assert hasattr(MockCommand1, "scopes") and MockCommand1.scopes == scopes # type: ignore + finally: + # Clean up - restore original attributes if they existed + if original_command is not None: + MockCommand1.command = original_command + elif hasattr(MockCommand1, "command"): + delattr(MockCommand1, "command") + + if original_scopes is not None: + MockCommand1.scopes = original_scopes + elif hasattr(MockCommand1, "scopes"): + delattr(MockCommand1, "scopes") From 3cdbcdd76e6f8426823cca53bf9877fc443a7cad Mon Sep 17 00:00:00 2001 From: Sandro Date: Tue, 27 Jan 2026 23:15:17 +0000 Subject: [PATCH 2/4] Fixed bugs. --- .../agent/middlewares/file_system.py | 4 +- daiv/automation/agent/middlewares/git.py | 1 + daiv/automation/agent/middlewares/sandbox.py | 2 +- daiv/automation/agent/middlewares/skills.py | 138 +++++------ .../agent/middlewares/web_search.py | 2 +- daiv/slash_commands/actions/clone_to_topic.py | 56 +---- daiv/slash_commands/actions/help.py | 50 ++-- daiv/slash_commands/base.py | 83 ++----- daiv/slash_commands/decorator.py | 3 - daiv/slash_commands/parser.py | 32 +-- .../slash_commands/error_message.txt | 9 - .../templates/slash_commands/invalid_args.txt | 10 - .../slash_commands/slash_commands_help.txt | 6 +- .../agent/middlewares/test_skills.py | 223 +++++++++++++++++- .../slash_commands/actions/test_help.py | 6 +- .../slash_commands/test_decorator.py | 2 +- .../unit_tests/slash_commands/test_parser.py | 71 ++---- 17 files changed, 373 insertions(+), 325 deletions(-) delete mode 100644 daiv/slash_commands/templates/slash_commands/error_message.txt delete mode 100644 daiv/slash_commands/templates/slash_commands/invalid_args.txt diff --git a/daiv/automation/agent/middlewares/file_system.py b/daiv/automation/agent/middlewares/file_system.py index 092683c3..35b9a578 100644 --- a/daiv/automation/agent/middlewares/file_system.py +++ b/daiv/automation/agent/middlewares/file_system.py @@ -30,10 +30,10 @@ DAIV_FILESYSTEM_SYSTEM_PROMPT = SystemMessagePromptTemplate.from_template( """\ -## Filesystem Tools `ls`, `read_file`, {{#read_only}}write_file`, `edit_file`, {{/read_only}}`glob`, `grep` +## Filesystem Tools You have access to a filesystem which you can interact with using these tools. -Tool-call arguments (ls/read_file{{#read_only}}/edit_file{{/read_only}}/etc.) MUST use absolute paths (start with "/"). +Tool-call arguments (ls/read_file{{^read_only}}/edit_file{{/read_only}}/etc.) MUST use absolute paths (start with "/"). User-visible output MUST NEVER contain "/repo/" and MUST use repo-relative paths. - ls: list files in a directory diff --git a/daiv/automation/agent/middlewares/git.py b/daiv/automation/agent/middlewares/git.py index 79870f4c..b7156e6c 100644 --- a/daiv/automation/agent/middlewares/git.py +++ b/daiv/automation/agent/middlewares/git.py @@ -43,6 +43,7 @@ The user will interact with you through the issue comments that will be automatically provided to you as messages. You should respond to the user's comments with the appropriate actions and tools. {{/issue_iid}} + {{#merge_request_iid}} You're currently working on merge request #{{merge_request_iid}}. diff --git a/daiv/automation/agent/middlewares/sandbox.py b/daiv/automation/agent/middlewares/sandbox.py index 7caca79f..9fd2a375 100644 --- a/daiv/automation/agent/middlewares/sandbox.py +++ b/daiv/automation/agent/middlewares/sandbox.py @@ -87,7 +87,7 @@ """ # noqa: E501 SANDBOX_SYSTEM_PROMPT = f"""\ -## Bash tool `{BASH_TOOL_NAME}` +## Bash tool You have access to a `{BASH_TOOL_NAME}` tool to execute bash commands on your working directory. Use this tool to run commands, scripts, tests, builds, and other shell operations. diff --git a/daiv/automation/agent/middlewares/skills.py b/daiv/automation/agent/middlewares/skills.py index 93bb5ad9..37cb08b5 100644 --- a/daiv/automation/agent/middlewares/skills.py +++ b/daiv/automation/agent/middlewares/skills.py @@ -14,13 +14,10 @@ from automation.agent.constants import BUILTIN_SKILLS_PATH, DAIV_SKILLS_PATH from automation.agent.utils import extract_body_from_frontmatter, extract_text_content from codebase.context import RuntimeCtx # noqa: TC001 -from slash_commands.parser import SlashCommandCommand, parse_agent_slash_command +from slash_commands.parser import SlashCommandCommand, parse_slash_command from slash_commands.registry import slash_command_registry if TYPE_CHECKING: - from collections.abc import Callable - - from deepagents.backends import BackendProtocol from langchain_core.runnables import RunnableConfig from langchain_core.tools import BaseTool @@ -80,60 +77,6 @@ ) -def _skill_tool_generator(backend: BackendProtocol | Callable[[ToolRuntime], BackendProtocol]) -> BaseTool: - """ - Generate a skill tool. - - Args: - backend: The backend to read the skill from. - - Returns: - A BaseTool. - """ - - async def skill_tool( - skill: Annotated[str, "The skill name. E.g. 'code-review' or 'web-research'"], - runtime: ToolRuntime[RuntimeCtx, SkillsState], - skill_args: Annotated[str | None, "Optional arguments to pass to the skill."] = None, - ) -> str: - """ - Tool to execute a skill. - """ - available_skills = runtime.state["skills_metadata"] - loaded_skill = next( - (skill_metadata for skill_metadata in available_skills if skill_metadata["name"] == skill), None - ) - - if loaded_skill is None: - available_skills_names = [skill_metadata["name"] for skill_metadata in available_skills] - return f"error: Skill '{skill}' not found. Available skills: {', '.join(available_skills_names)}." - - responses = backend.download_files([loaded_skill["path"]]) - if responses[0].error: - return f"error: Failed to launch skill '{skill}': {responses[0].error}. {responses[0].error_message}" - - body = extract_body_from_frontmatter(responses[0].content.decode("utf-8").strip()) - - # Positional args like $1, $2 - for i, a in enumerate(shlex.split(skill_args or ""), start=1): - body = body.replace(f"${i}", a).replace(f"{SKILL_ARGUMENTS_PLACEHOLDER}[{i}]", a) - - # Named args, only $ARGUMENTS supported - if arg_str := skill_args.strip(): - body = ( - body.replace(SKILL_ARGUMENTS_PLACEHOLDER, arg_str) - if SKILL_ARGUMENTS_PLACEHOLDER in body - else f"{body}\n\n{SKILL_ARGUMENTS_PLACEHOLDER}: {arg_str}" - ) - - return [ - ToolMessage(content=f"Launching skill '{skill}'...", tool_call_id=runtime.tool_call_id), - HumanMessage(content=body), - ] - - return tool(SKILLS_TOOL_NAME, description=SKILLS_TOOL_DESCRIPTION)(skill_tool) - - class SkillsMiddleware(DeepAgentsSkillsMiddleware): """ Middleware to apply builtin slash commands early in the conversation and copy builtin skills to the project skills @@ -143,7 +86,7 @@ class SkillsMiddleware(DeepAgentsSkillsMiddleware): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.system_prompt_template = SKILLS_SYSTEM_PROMPT - self.tools = [_skill_tool_generator(self._backend)] + self.tools = [self._skill_tool_generator()] @hook_config(can_jump_to=["end"]) async def abefore_agent( @@ -153,10 +96,6 @@ async def abefore_agent( Apply builtin slash commands early in the conversation and copy builtin skills to the project skills directory to make them available to the agent. """ - builtin_slash_commands = await self._apply_builtin_slash_commands(state["messages"], runtime.context) - if builtin_slash_commands: - return builtin_slash_commands - if "skills_metadata" in state: return None @@ -172,6 +111,14 @@ async def abefore_agent( skill["metadata"]["is_builtin"] = True else: skill["metadata"].pop("is_builtin", None) + + builtin_slash_commands = await self._apply_builtin_slash_commands( + state["messages"], runtime.context, skills_update["skills_metadata"] + ) + + if builtin_slash_commands: + return builtin_slash_commands + return skills_update async def _copy_builtin_skills(self, agent_path: Path) -> list[str]: @@ -234,7 +181,7 @@ def _format_skills_list(self, skills: list[SkillMetadata]) -> str: return AVAILABLE_SKILLS_TEMPLATE.format(skills_list=skills) async def _apply_builtin_slash_commands( - self, messages: list[AnyMessage], context: RuntimeCtx + self, messages: list[AnyMessage], context: RuntimeCtx, skills: list[SkillMetadata] ) -> SkillsStateUpdate | None: """ Detect and execute builtin slash commands (not project skills) early in the conversation. @@ -242,6 +189,7 @@ async def _apply_builtin_slash_commands( Args: messages: The list of messages. context: The runtime context. + skills: The list of skills. Returns: State update with messages injected, or None if no builtin slash command detected. @@ -264,17 +212,15 @@ async def _apply_builtin_slash_commands( ) return None - command = command_classes[0]() + command = command_classes[0](scope=context.scope, repo_id=context.repo_id, bot_username=context.bot_username) logger.info("[%s] Executing `%s` slash command", self.name, slash_command.raw) try: result = await command.execute_for_agent( args=" ".join(slash_command.args), - scope=context.scope, - repo_id=context.repo_id, - bot_username=context.bot_username, issue_iid=context.issue.iid if context.issue else None, merge_request_id=context.merge_request.merge_request_id if context.merge_request else None, + available_skills=skills, ) except Exception: logger.exception("[%s] Failed to execute `%s` slash command", self.name, slash_command.raw) @@ -303,4 +249,58 @@ def _extract_slash_command(self, messages: list[AnyMessage], bot_username: str) if not text_content or not text_content.strip(): return None - return parse_agent_slash_command(text_content, bot_username) + return parse_slash_command(text_content, bot_username) + + def _skill_tool_generator(self) -> BaseTool: + """ + Generate a skill tool. + + Args: + backend: The backend to read the skill from. + + Returns: + A BaseTool. + """ + + async def skill_tool( + skill: Annotated[str, "The skill name. E.g. 'code-review' or 'web-research'"], + runtime: ToolRuntime[RuntimeCtx, SkillsState], + skill_args: Annotated[str | None, "Optional arguments to pass to the skill."] = None, + ) -> str: + """ + Tool to execute a skill. + """ + available_skills = runtime.state["skills_metadata"] + + loaded_skill = next( + (skill_metadata for skill_metadata in available_skills if skill_metadata["name"] == skill), None + ) + + if loaded_skill is None: + available_skills_names = [skill_metadata["name"] for skill_metadata in available_skills] + return f"error: Skill '{skill}' not found. Available skills: {', '.join(available_skills_names)}." + + responses = await self._backend.adownload_files([loaded_skill["path"]]) + if responses[0].error: + return f"error: Failed to launch skill '{skill}': {responses[0].error}." + + body = extract_body_from_frontmatter(responses[0].content.decode("utf-8").strip()) + + # Positional args like $1, $2 + for i, a in enumerate(shlex.split(skill_args or ""), start=1): + body = body.replace(f"${i}", a).replace(f"{SKILL_ARGUMENTS_PLACEHOLDER}[{i}]", a) + + # Named args, only $ARGUMENTS supported + if skill_args and (arg_str := skill_args.strip()): + body = ( + body.replace(SKILL_ARGUMENTS_PLACEHOLDER, arg_str) + if SKILL_ARGUMENTS_PLACEHOLDER in body + else f"{body}\n\n{SKILL_ARGUMENTS_PLACEHOLDER}: {arg_str}" + ) + + return [ + ToolMessage(content=f"Launching skill '{skill}'...", tool_call_id=runtime.tool_call_id), + HumanMessage(content=body), + ] + + return tool(SKILLS_TOOL_NAME, description=SKILLS_TOOL_DESCRIPTION)(skill_tool) diff --git a/daiv/automation/agent/middlewares/web_search.py b/daiv/automation/agent/middlewares/web_search.py index 314817b3..84e1a9e2 100644 --- a/daiv/automation/agent/middlewares/web_search.py +++ b/daiv/automation/agent/middlewares/web_search.py @@ -34,7 +34,7 @@ WEB_SEARCH_SYSTEM_PROMPT = f"""\ -## Web Search tool `{WEB_SEARCH_NAME}` +## Web Search tool You have access to a `{WEB_SEARCH_NAME}` tool to allow you to search the web and use the results to inform your responses. diff --git a/daiv/slash_commands/actions/clone_to_topic.py b/daiv/slash_commands/actions/clone_to_topic.py index 607eb6cc..e403e116 100644 --- a/daiv/slash_commands/actions/clone_to_topic.py +++ b/daiv/slash_commands/actions/clone_to_topic.py @@ -1,5 +1,4 @@ from contextlib import suppress -from typing import TYPE_CHECKING from django.template.loader import render_to_string @@ -7,9 +6,6 @@ from slash_commands.base import SlashCommand from slash_commands.decorator import slash_command -if TYPE_CHECKING: - from codebase.base import Discussion, Issue - @slash_command(command="clone-to-topic", scopes=[Scope.ISSUE]) class CloneToTopicSlashCommand(SlashCommand): @@ -31,47 +27,6 @@ def validate_arguments(self, args: str) -> bool: """ return bool(args.strip()) - async def execute_action_for_issue(self, repo_id: str, *, args: str, comment: Discussion, issue: Issue) -> None: - """ - Clone the issue to all repositories matching the specified topics. - - Args: - repo_id: The repository ID. - comment: The comment that triggered the command. - issue: The issue where the command was triggered. - args: Comma-separated list of topics. - """ - # Parse topics from args - topics = [topic.strip() for topic in args.split(",") if topic.strip()] - - if not topics: - self._add_invalid_args_message(repo_id, issue.iid, comment.id, args, scope=Scope.ISSUE) - return - - target_repos = [repo for repo in self.client.list_repositories(topics=topics) if repo.slug != repo_id] - - if not target_repos: - topics_str = ", ".join([f"`{topic}`" for topic in topics]) - message = f"No repositories matching the specified topics {topics_str} found." - self.client.create_issue_comment(repo_id, issue.iid, message, reply_to_id=comment.id) - return - - cloned_issues = [] - - for target_repo in target_repos: - with suppress(Exception): - cloned_issue_iid = self.client.create_issue( - repo_id=target_repo.slug, title=issue.title, description=issue.description, labels=issue.labels - ) - - cloned_issues.append(f"{target_repo.slug}#{cloned_issue_iid}") - - note_message = render_to_string( - "slash_commands/clone_to_topic_result.txt", - {"total_count": len(cloned_issues), "cloned_issues": cloned_issues}, - ) - self.client.create_issue_comment(repo_id, issue.iid, note_message, reply_to_id=comment.id) - async def execute_for_agent( self, *, @@ -87,27 +42,24 @@ async def execute_for_agent( Returns a message listing the cloned issues or an error message. """ - if not self.validate_arguments(args): - return f"Invalid arguments for /{self.command}. Please provide comma-separated topics." - if scope != Scope.ISSUE or issue_iid is None: return f"The /{self.command} command is only available for issues." - # Parse topics from args + if not self.validate_arguments(args): + return f"Invalid arguments for /{self.command}. Please provide comma-separated topics." + topics = [topic.strip() for topic in args.split(",") if topic.strip()] if not topics: return f"Invalid arguments for /{self.command}. Please provide comma-separated topics." - # Get the issue details - issue = self.client.get_issue(repo_id, issue_iid) - target_repos = [repo for repo in self.client.list_repositories(topics=topics) if repo.slug != repo_id] if not target_repos: topics_str = ", ".join([f"`{topic}`" for topic in topics]) return f"No repositories matching the specified topics {topics_str} found." + issue = self.client.get_issue(repo_id, issue_iid) cloned_issues = [] for target_repo in target_repos: diff --git a/daiv/slash_commands/actions/help.py b/daiv/slash_commands/actions/help.py index dfdcf7f6..44aa77c5 100644 --- a/daiv/slash_commands/actions/help.py +++ b/daiv/slash_commands/actions/help.py @@ -1,3 +1,5 @@ +from typing import TYPE_CHECKING + from django.template.loader import render_to_string from codebase.base import Scope @@ -6,6 +8,9 @@ from slash_commands.decorator import slash_command from slash_commands.registry import slash_command_registry +if TYPE_CHECKING: + from deepagents.middleware.skills import SkillMetadata + @slash_command(command="help", scopes=[Scope.GLOBAL, Scope.ISSUE, Scope.MERGE_REQUEST]) class HelpSlashCommand(SlashCommand): @@ -15,34 +20,45 @@ class HelpSlashCommand(SlashCommand): description: str = "Shows the help message with the available slash commands." - async def execute_for_agent( - self, - *, - args: str, - scope: Scope, - repo_id: str, - bot_username: str, - issue_iid: int | None = None, - merge_request_id: int | None = None, - ) -> str: + async def execute_for_agent(self, *, args: str, available_skills: list[SkillMetadata], **kwargs) -> str: """ Execute help command for agent middleware. Args: args: Additional parameters from the command. - scope: The scope to get the help message for. - repo_id: The repository ID. - bot_username: The bot username. - issue_iid: The issue IID (for Issue scope). - merge_request_id: The merge request ID (for Merge Request scope). + available_skills: The list of available skills. Returns: The help message for the given scope. """ - commands_help = [command().help() for command in slash_command_registry.get_commands(scope=scope)] + commands_help = [ + command(scope=self.scope, repo_id=self.repo_id, bot_username=self.bot_username).help() + for command in slash_command_registry.get_commands(scope=self.scope) + ] + + commands_help += [self._format_skill_help(skill) for skill in available_skills] + if not commands_help: return "No slash commands available." + return render_to_string( "slash_commands/slash_commands_help.txt", - {"bot_name": BOT_NAME, "scope": scope.value.lower(), "actions": commands_help}, + { + "bot_name": BOT_NAME, + "need_mention_to_invoke": self.need_mention, + "bot_username": self.bot_username, + "actions": commands_help, + }, ) + + def _format_skill_help(self, skill: SkillMetadata) -> str: + """ + Format the help message for a skill. + + Args: + skill: The skill metadata. + + Returns: + The help message for the skill. + """ + return f" * `/{skill['name']}` - {skill['description']}" diff --git a/daiv/slash_commands/base.py b/daiv/slash_commands/base.py index a6bdaff8..34844ee2 100644 --- a/daiv/slash_commands/base.py +++ b/daiv/slash_commands/base.py @@ -2,10 +2,7 @@ from abc import ABC -from django.template.loader import render_to_string - from codebase.base import Scope -from codebase.clients import RepoClient class SlashCommand(ABC): @@ -17,32 +14,33 @@ class SlashCommand(ABC): scopes: list[Scope] description: str - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.client = RepoClient.create_instance() + def __init__(self, *, scope: Scope, repo_id: str, bot_username: str | None = None): + self.scope = scope + self.repo_id = repo_id + self.bot_username = bot_username - def help(self) -> str: + @property + def command_to_invoke(self) -> str: """ - Get the help message for the command. + Get the command to activate the action. """ - return f" * `{self.command_to_activate}` - {self.description}" + return f"/{self.command}" @property - def command_to_activate(self) -> str: + def need_mention(self) -> bool: """ - Get the command to activate the action. + Check if the command needs to be mentioned. """ - return f"@{self.client.current_user.username} /{self.command}" + return self.scope != Scope.GLOBAL and self.bot_username + + def help(self) -> str: + """ + Get the help message for the command. + """ + return f" * `{self.command_to_invoke}` - {self.description}" async def execute_for_agent( - self, - *, - args: str, - scope: Scope, - repo_id: str, - bot_username: str, - issue_iid: int | None = None, - merge_request_id: int | None = None, + self, *, args: str, issue_iid: int | None = None, merge_request_id: int | None = None ) -> str: """ Execute the slash command for agent middleware. @@ -52,9 +50,6 @@ async def execute_for_agent( Args: args: Additional parameters from the command. - scope: The scope (Issue or Merge Request). - repo_id: The repository ID. - bot_username: The bot username. issue_iid: The issue IID (for Issue scope). merge_request_id: The merge request ID (for Merge Request scope). @@ -62,45 +57,3 @@ async def execute_for_agent( The result content. """ raise NotImplementedError("execute_for_agent is not implemented") - - def validate_arguments(self, args: str) -> bool: - """ - Validate the arguments are valid. - - Args: - args: The arguments to validate. - - Returns: - bool: True if the arguments are valid, False otherwise. - """ - return True - - def _add_invalid_args_message( - self, repo_id: str, object_id: int, comment_id: str, invalid_args: str, scope: Scope - ) -> None: - """ - Add an invalid arguments message to the merge request discussion. - - Args: - repo_id: The repository ID. - object_id: The merge request or issue ID. - comment_id: The comment ID of the note. - invalid_args: The invalid arguments. - scope: The scope of the slash command. - """ - note_message = render_to_string( - "slash_commands/invalid_args.txt", - { - "bot_name": self.client.current_user.username, - "command": self.command, - "help": self.help(), - "invalid_args": invalid_args, - }, - ) - - if scope == Scope.MERGE_REQUEST: - self.client.create_merge_request_comment( - repo_id, object_id, note_message, reply_to_id=comment_id, mark_as_resolved=True - ) - elif scope == Scope.ISSUE: - self.client.create_issue_comment(repo_id, object_id, note_message, reply_to_id=comment_id) diff --git a/daiv/slash_commands/decorator.py b/daiv/slash_commands/decorator.py index e85f4aa8..28de9f7d 100644 --- a/daiv/slash_commands/decorator.py +++ b/daiv/slash_commands/decorator.py @@ -31,6 +31,3 @@ def decorator(cls: type[SlashCommand]) -> type[SlashCommand]: return cls return decorator - - -__all__ = ["slash_command"] diff --git a/daiv/slash_commands/parser.py b/daiv/slash_commands/parser.py index 7863a91a..a4091b01 100644 --- a/daiv/slash_commands/parser.py +++ b/daiv/slash_commands/parser.py @@ -22,6 +22,8 @@ class SlashCommandCommand: (?P[^\n\r]+) # capture the rest of the line (until newline) """ +_BARE_COMMAND_RE_TEMPLATE = r"^\s*/(?P[^\n\r]+)" + def _parse_command_match(text: str, *, pattern: str, flags: int) -> SlashCommandCommand | None: match = re.search(pattern, text, flags=flags) @@ -43,7 +45,7 @@ def _parse_command_match(text: str, *, pattern: str, flags: int) -> SlashCommand def parse_slash_command(note_body: str, bot_name: str) -> SlashCommandCommand | None: """ - Parse the first '@ …' command in `note_body`. + Parse the first '@ / [arguments]' or '/ [arguments]' command in `note_body`. Args: note_body: The full text of a GitLab note / comment. @@ -52,29 +54,11 @@ def parse_slash_command(note_body: str, bot_name: str) -> SlashCommandCommand | Returns: SlashCommandCommand if found, otherwise None. """ - pattern = _COMMAND_RE_TEMPLATE.format(bot=re.escape(bot_name)) - - return _parse_command_match(note_body, pattern=pattern, flags=re.IGNORECASE | re.VERBOSE) - - -def parse_agent_slash_command(text: str, bot_name: str) -> SlashCommandCommand | None: - """ - Parse slash commands for agent middleware. - - Supports both mention-based format (`@ /command ...`) and bare slash commands (`/command ...`). - - Args: - text: The message text to parse. - bot_name: The bot mention to look for (case-insensitive). - - Returns: - SlashCommandCommand if found, otherwise None. - """ - # Try mention-based format first - if result := parse_slash_command(text, bot_name): + # Try mention-based format + if result := _parse_command_match( + note_body, pattern=_COMMAND_RE_TEMPLATE.format(bot=re.escape(bot_name)), flags=re.IGNORECASE | re.VERBOSE + ): return result # Try bare slash command format - # Look for lines starting with '/' (optionally preceded by whitespace) - bare_pattern = r"^\s*/(?P[^\n\r]+)" - return _parse_command_match(text, pattern=bare_pattern, flags=re.MULTILINE) + return _parse_command_match(note_body, pattern=_BARE_COMMAND_RE_TEMPLATE, flags=re.IGNORECASE) diff --git a/daiv/slash_commands/templates/slash_commands/error_message.txt b/daiv/slash_commands/templates/slash_commands/error_message.txt deleted file mode 100644 index ea79d256..00000000 --- a/daiv/slash_commands/templates/slash_commands/error_message.txt +++ /dev/null @@ -1,9 +0,0 @@ -### ❌ Slash Command Error - -I tried to run **`{{ command }}`**, but something unexpected happened and the command didn't complete. - -**What you can do now** - -1. 🔄 **Retry** - add the same slash command again. -2. 📜 **Check the app logs** - open the DAIV logs to see the full stack trace and [open an issue](https://github.com/srtab/daiv/issues/new) if the problem persists. - diff --git a/daiv/slash_commands/templates/slash_commands/invalid_args.txt b/daiv/slash_commands/templates/slash_commands/invalid_args.txt deleted file mode 100644 index 8a5343ba..00000000 --- a/daiv/slash_commands/templates/slash_commands/invalid_args.txt +++ /dev/null @@ -1,10 +0,0 @@ -### ⚠️ Invalid Arguments for Slash Command - -`@{{ bot_name }} /{{ command }} {{ invalid_args }}` aren't recognised arguments for **/{{ command }}**. - -**Here's how to use it correctly:** - -{{ help }} - -Need more options? Comment **`@{{ bot_name }} /help`** to see the full slash command reference. - diff --git a/daiv/slash_commands/templates/slash_commands/slash_commands_help.txt b/daiv/slash_commands/templates/slash_commands/slash_commands_help.txt index 4b9ab9c0..8cca6350 100644 --- a/daiv/slash_commands/templates/slash_commands/slash_commands_help.txt +++ b/daiv/slash_commands/templates/slash_commands/slash_commands_help.txt @@ -1,6 +1,8 @@ ### 🤖 {{ bot_name }} Slash Commands -Comment **one** of the commands below on this {{ scope }} to trigger the bot: {% for action in actions %}{{ action }} -{% endfor %} +{% endfor -%} +You can **invoke slash commands**, either by: +- Using the slash command: `{% if need_mention_to_invoke %}@{{ bot_username }} {% endif %}/command-name [arguments]` +- Asking me to perform a task that matches the command's description (I'll automatically invoke the appropriate command) \ No newline at end of file diff --git a/tests/unit_tests/automation/agent/middlewares/test_skills.py b/tests/unit_tests/automation/agent/middlewares/test_skills.py index 02a73d0b..f4cb43b2 100644 --- a/tests/unit_tests/automation/agent/middlewares/test_skills.py +++ b/tests/unit_tests/automation/agent/middlewares/test_skills.py @@ -4,15 +4,26 @@ from unittest.mock import AsyncMock, Mock, patch import pytest +from langchain_core.messages import AIMessage, HumanMessage, ToolMessage from automation.agent.middlewares.skills import SkillsMiddleware +from codebase.base import Scope +from slash_commands.base import SlashCommand +from slash_commands.registry import SlashCommandRegistry -def _make_runtime(*, repo_working_dir: str) -> Mock: +def _make_runtime( + *, repo_working_dir: str, bot_username: str = "daiv-bot", scope: Scope = Scope.GLOBAL, repo_id: str = "repo-1" +) -> Mock: runtime = Mock() runtime.context = Mock() runtime.context.repo = Mock() runtime.context.repo.working_dir = repo_working_dir + runtime.context.bot_username = bot_username + runtime.context.scope = scope + runtime.context.repo_id = repo_id + runtime.context.issue = None + runtime.context.merge_request = None return runtime @@ -64,7 +75,7 @@ async def test_copies_builtin_skills_then_delegates_to_super(self, tmp_path: Pat runtime = _make_runtime(repo_working_dir=str(tmp_path / repo_name)) with patch("automation.agent.middlewares.skills.BUILTIN_SKILLS_PATH", builtin): - result = await middleware.abefore_agent({}, runtime, Mock()) + result = await middleware.abefore_agent({"messages": [HumanMessage(content="hello")]}, runtime, Mock()) assert result is not None skills = {skill["name"]: skill for skill in result["skills_metadata"]} @@ -99,7 +110,7 @@ async def test_marks_builtin_metadata_and_clears_custom(self, tmp_path: Path): runtime = _make_runtime(repo_working_dir=str(tmp_path / repo_name)) with patch("automation.agent.middlewares.skills.BUILTIN_SKILLS_PATH", builtin): - result = await middleware.abefore_agent({}, runtime, Mock()) + result = await middleware.abefore_agent({"messages": [HumanMessage(content="hello")]}, runtime, Mock()) assert result is not None skills = {skill["name"]: skill for skill in result["skills_metadata"]} @@ -212,8 +223,204 @@ def test_format_skills_list_marks_builtin(self): }, ]) - lines = formatted.splitlines() - assert lines[0] == "- **skill-one (Builtin)**: does one" - assert lines[1] == " -> Read `/skills/skill-one/SKILL.md` for full instructions" - assert lines[2] == "- **custom-skill**: does custom" - assert lines[3] == " -> Read `/skills/custom-skill/SKILL.md` for full instructions" + assert formatted.startswith("") + assert "skill-one" in formatted + assert "does one" in formatted + assert "custom-skill" in formatted + assert "does custom" in formatted + assert formatted.count("true") == 1 + + def test_format_skills_list_returns_empty_hint(self): + middleware = SkillsMiddleware(backend=Mock(), sources=["/skills", "/extra/skills"]) + formatted = middleware._format_skills_list([]) + assert formatted == "(No skills available yet. You can create skills in /skills or /extra/skills)" + + def test_extract_slash_command_requires_human_message(self): + middleware = SkillsMiddleware(backend=Mock(), sources=["/skills"]) + messages = [AIMessage(content="hello")] + assert middleware._extract_slash_command(messages, "daiv") is None + + def test_extract_slash_command_skips_blank_content(self): + middleware = SkillsMiddleware(backend=Mock(), sources=["/skills"]) + messages = [HumanMessage(content=" \n\t ")] + assert middleware._extract_slash_command(messages, "daiv") is None + + def test_extract_slash_command_parses_multimodal_content(self): + middleware = SkillsMiddleware(backend=Mock(), sources=["/skills"]) + messages = [ + HumanMessage( + content=[ + {"type": "text", "text": "@daiv /help arg1"}, + {"type": "image_url", "image_url": {"url": "https://example.com/demo.png"}}, + ] + ) + ] + result = middleware._extract_slash_command(messages, "daiv") + assert result is not None + assert result.command == "help" + assert result.args == ["arg1"] + assert result.raw == "@daiv /help arg1" + + async def test_apply_builtin_slash_commands_executes_command(self): + class DemoSlashCommand(SlashCommand): + description = "demo" + + async def execute_for_agent( + self, + *, + args: str, + issue_iid: int | None = None, + merge_request_id: int | None = None, + available_skills: list | None = None, + ) -> str: + skill_name = available_skills[0]["name"] if available_skills else "none" + return f"{args}|{issue_iid}|{merge_request_id}|{skill_name}" + + registry = SlashCommandRegistry() + registry.register(DemoSlashCommand, "demo", [Scope.GLOBAL]) + + middleware = SkillsMiddleware(backend=Mock(), sources=["/skills"]) + context = Mock() + context.bot_username = "daiv" + context.scope = Scope.GLOBAL + context.repo_id = "repo-1" + context.issue = Mock(iid=101) + context.merge_request = Mock(merge_request_id=202) + + with patch("automation.agent.middlewares.skills.slash_command_registry", registry): + result = await middleware._apply_builtin_slash_commands( + [HumanMessage(content="/demo arg1")], context, [{"name": "skill-one"}] + ) + + assert result is not None + assert result["jump_to"] == "end" + assert isinstance(result["messages"][0], AIMessage) + assert result["messages"][0].content == "arg1|101|202|skill-one" + + async def test_apply_builtin_slash_commands_returns_error_message_on_failure(self): + class FailingSlashCommand(SlashCommand): + description = "fail" + + async def execute_for_agent( + self, + *, + args: str, + issue_iid: int | None = None, + merge_request_id: int | None = None, + available_skills: list | None = None, + ) -> str: + raise RuntimeError("boom") + + registry = SlashCommandRegistry() + registry.register(FailingSlashCommand, "fail", [Scope.GLOBAL]) + + middleware = SkillsMiddleware(backend=Mock(), sources=["/skills"]) + context = Mock() + context.bot_username = "daiv" + context.scope = Scope.GLOBAL + context.repo_id = "repo-1" + context.issue = None + context.merge_request = None + + with patch("automation.agent.middlewares.skills.slash_command_registry", registry): + result = await middleware._apply_builtin_slash_commands( + [HumanMessage(content="/fail now")], context, [{"name": "skill-one"}] + ) + + assert result is not None + assert result["jump_to"] == "end" + assert isinstance(result["messages"][0], AIMessage) + assert result["messages"][0].content == "Failed to execute `/fail now`." + + async def test_apply_builtin_slash_commands_returns_none_for_ambiguous_command(self): + class DemoSlashCommand(SlashCommand): + description = "demo" + + class OtherSlashCommand(SlashCommand): + description = "other" + + DemoSlashCommand.command = "demo" + OtherSlashCommand.command = "demo" + + registry = Mock() + registry.get_commands.return_value = [DemoSlashCommand, OtherSlashCommand] + + middleware = SkillsMiddleware(backend=Mock(), sources=["/skills"]) + context = Mock() + context.bot_username = "daiv" + context.scope = Scope.GLOBAL + context.repo_id = "repo-1" + context.issue = None + context.merge_request = None + + with patch("automation.agent.middlewares.skills.slash_command_registry", registry): + result = await middleware._apply_builtin_slash_commands( + [HumanMessage(content="/demo now")], context, [{"name": "skill-one"}] + ) + + assert result is None + + async def test_skill_tool_reports_missing_skill(self): + backend = Mock() + middleware = SkillsMiddleware(backend=backend, sources=["/skills"]) + tool = middleware._skill_tool_generator() + + runtime = Mock() + runtime.state = {"skills_metadata": [{"name": "demo", "path": "/skills/demo/SKILL.md"}]} + runtime.tool_call_id = "call_1" + + result = await tool.coroutine(skill="missing", runtime=runtime) # type: ignore[union-attr] + assert result == "error: Skill 'missing' not found. Available skills: demo." + + async def test_skill_tool_reports_download_failure(self): + backend = Mock() + backend.adownload_files = AsyncMock(return_value=[Mock(error="boom", content=b"")]) + middleware = SkillsMiddleware(backend=backend, sources=["/skills"]) + tool = middleware._skill_tool_generator() + + runtime = Mock() + runtime.state = {"skills_metadata": [{"name": "demo", "path": "/skills/demo/SKILL.md"}]} + runtime.tool_call_id = "call_1" + + result = await tool.coroutine(skill="demo", runtime=runtime) # type: ignore[union-attr] + assert result == "error: Failed to launch skill 'demo': boom." + + async def test_skill_tool_formats_body_with_arguments(self): + backend = Mock() + backend.adownload_files = AsyncMock( + return_value=[ + Mock( + error=None, + content=(b"---\nname: demo\ndescription: Demo\n---\nFirst $1, second $2, all: $ARGUMENTS"), + ) + ] + ) + middleware = SkillsMiddleware(backend=backend, sources=["/skills"]) + tool = middleware._skill_tool_generator() + + runtime = Mock() + runtime.state = {"skills_metadata": [{"name": "demo", "path": "/skills/demo/SKILL.md"}]} + runtime.tool_call_id = "call_1" + + result = await tool.coroutine(skill="demo", runtime=runtime, skill_args="alpha beta") # type: ignore[union-attr] + assert isinstance(result, list) + assert isinstance(result[0], ToolMessage) + assert result[0].content == "Launching skill 'demo'..." + assert isinstance(result[1], HumanMessage) + assert result[1].content == "First alpha, second beta, all: alpha beta" + + async def test_skill_tool_appends_named_arguments_when_missing_placeholder(self): + backend = Mock() + backend.adownload_files = AsyncMock( + return_value=[Mock(error=None, content=b"---\nname: demo\ndescription: Demo\n---\nRun this.")] + ) + middleware = SkillsMiddleware(backend=backend, sources=["/skills"]) + tool = middleware._skill_tool_generator() + + runtime = Mock() + runtime.state = {"skills_metadata": [{"name": "demo", "path": "/skills/demo/SKILL.md"}]} + runtime.tool_call_id = "call_1" + + result = await tool.coroutine(skill="demo", runtime=runtime, skill_args="--flag=1") # type: ignore[union-attr] + assert isinstance(result, list) + assert result[1].content.endswith("\n\n$ARGUMENTS: --flag=1") diff --git a/tests/unit_tests/slash_commands/actions/test_help.py b/tests/unit_tests/slash_commands/actions/test_help.py index 750e8123..bc9331f6 100644 --- a/tests/unit_tests/slash_commands/actions/test_help.py +++ b/tests/unit_tests/slash_commands/actions/test_help.py @@ -9,7 +9,7 @@ @fixture def help_slash_command() -> HelpSlashCommand: """Set up test fixtures.""" - command = HelpSlashCommand() + command = HelpSlashCommand(scope=Scope.ISSUE, repo_id="repo1", bot_username="bot") command.client = MagicMock(current_user=MagicMock(username="bot")) return command @@ -26,7 +26,5 @@ def test_help_command_has_correct_attributes(): @patch("slash_commands.actions.help.slash_command_registry.get_commands", new=Mock(return_value=[])) async def test_help_command_returns_correct_message(help_slash_command: HelpSlashCommand): """Test that HelpSlashCommand returns the correct message.""" - message = await help_slash_command.execute_for_agent( - args="", scope=Scope.ISSUE, repo_id="repo1", bot_username="bot" - ) + message = await help_slash_command.execute_for_agent(args="", available_skills=[]) assert message == "No slash commands available." diff --git a/tests/unit_tests/slash_commands/test_decorator.py b/tests/unit_tests/slash_commands/test_decorator.py index 3c30dd84..ae9d9ffe 100644 --- a/tests/unit_tests/slash_commands/test_decorator.py +++ b/tests/unit_tests/slash_commands/test_decorator.py @@ -79,5 +79,5 @@ class InheritedCommand(BaseCommand): mock_registry.register.assert_called_once_with(InheritedCommand, "inherited_command", [Scope.ISSUE]) # Verify inheritance still works - command = InheritedCommand() + command = InheritedCommand(scope=Scope.ISSUE, repo_id="repo1", bot_username="bot") assert command.shared_method() == "shared" diff --git a/tests/unit_tests/slash_commands/test_parser.py b/tests/unit_tests/slash_commands/test_parser.py index af30ed67..94b849eb 100644 --- a/tests/unit_tests/slash_commands/test_parser.py +++ b/tests/unit_tests/slash_commands/test_parser.py @@ -1,6 +1,6 @@ import pytest -from slash_commands.parser import parse_agent_slash_command, parse_slash_command +from slash_commands.parser import parse_slash_command class TestParseSlashCommand: @@ -162,34 +162,10 @@ def test_parse_whitespace_only_note_body(self): result = parse_slash_command(" \n\t ", "testbot") assert result is None - -class TestParseAgentSlashCommand: - """Test suite for parse_agent_slash_command that supports both mention and bare slash commands.""" - - def test_parse_mention_format(self): - """Test parsing mention-based format (@bot /command).""" - text = "@testbot /help" - result = parse_agent_slash_command(text, "testbot") - - assert result is not None - assert result.command == "help" - assert result.args == [] - assert result.raw == "@testbot /help" - - def test_parse_mention_format_with_args(self): - """Test parsing mention-based format with arguments.""" - text = "@testbot /clone-to-topic backend, api" - result = parse_agent_slash_command(text, "testbot") - - assert result is not None - assert result.command == "clone-to-topic" - assert result.args == ["backend,", "api"] - assert result.raw == "@testbot /clone-to-topic backend, api" - def test_parse_bare_slash_command(self): """Test parsing bare slash command (/command).""" text = "/help" - result = parse_agent_slash_command(text, "testbot") + result = parse_slash_command(text, "testbot") assert result is not None assert result.command == "help" @@ -199,7 +175,7 @@ def test_parse_bare_slash_command(self): def test_parse_bare_slash_command_with_args(self): """Test parsing bare slash command with arguments.""" text = "/review please check the security aspects" - result = parse_agent_slash_command(text, "testbot") + result = parse_slash_command(text, "testbot") assert result is not None assert result.command == "review" @@ -209,7 +185,7 @@ def test_parse_bare_slash_command_with_args(self): def test_parse_bare_slash_command_with_leading_whitespace(self): """Test parsing bare slash command with leading whitespace.""" text = " /help" - result = parse_agent_slash_command(text, "testbot") + result = parse_slash_command(text, "testbot") assert result is not None assert result.command == "help" @@ -219,16 +195,14 @@ def test_parse_bare_slash_command_with_leading_whitespace(self): def test_parse_bare_slash_command_in_multiline(self): """Test parsing bare slash command in multiline text.""" text = "Some text before\n/help\nSome text after" - result = parse_agent_slash_command(text, "testbot") + result = parse_slash_command(text, "testbot") - assert result is not None - assert result.command == "help" - assert result.args == [] + assert result is None def test_parse_prioritizes_mention_over_bare(self): """Test that mention format is prioritized over bare format.""" text = "/bare-command\n@testbot /mention-command" - result = parse_agent_slash_command(text, "testbot") + result = parse_slash_command(text, "testbot") # Should find the mention format first assert result is not None @@ -237,23 +211,16 @@ def test_parse_prioritizes_mention_over_bare(self): def test_parse_bare_slash_command_with_quoted_args(self): """Test parsing bare slash command with quoted arguments.""" text = '/security-audit "check authentication"' - result = parse_agent_slash_command(text, "testbot") + result = parse_slash_command(text, "testbot") assert result is not None assert result.command == "security-audit" assert result.args == ["check authentication"] - def test_parse_no_command_found(self): - """Test when no command is found.""" - text = "Just regular text without any commands" - result = parse_agent_slash_command(text, "testbot") - - assert result is None - def test_parse_bare_slash_at_start_of_line(self): """Test parsing bare slash command at the start of a line.""" text = "/help me understand" - result = parse_agent_slash_command(text, "testbot") + result = parse_slash_command(text, "testbot") assert result is not None assert result.command == "help" @@ -262,39 +229,29 @@ def test_parse_bare_slash_at_start_of_line(self): def test_parse_bare_slash_not_mid_word(self): """Test that slash in middle of word is not detected.""" text = "http://example.com/help" - result = parse_agent_slash_command(text, "testbot") + result = parse_slash_command(text, "testbot") assert result is None def test_parse_bare_slash_command_case_insensitive(self): """Test that bare slash commands are converted to lowercase.""" text = "/HELP" - result = parse_agent_slash_command(text, "testbot") + result = parse_slash_command(text, "testbot") assert result is not None assert result.command == "help" - def test_parse_empty_text(self): - """Test parsing empty text.""" - result = parse_agent_slash_command("", "testbot") - assert result is None - - def test_parse_whitespace_only(self): - """Test parsing whitespace-only text.""" - result = parse_agent_slash_command(" \n\t ", "testbot") - assert result is None - def test_parse_bare_slash_shlex_error(self): """Test handling of shlex parsing errors in bare format.""" text = '/command "unmatched quote' - result = parse_agent_slash_command(text, "testbot") + result = parse_slash_command(text, "testbot") assert result is None def test_parse_bare_slash_only(self): """Test parsing when only slash is present.""" text = "/" - result = parse_agent_slash_command(text, "testbot") + result = parse_slash_command(text, "testbot") # Should return None as there's no command after the slash assert result is None @@ -302,7 +259,7 @@ def test_parse_bare_slash_only(self): def test_parse_bare_slash_with_tabs(self): """Test parsing bare slash command with tabs.""" text = "/help\targ1\targ2" - result = parse_agent_slash_command(text, "testbot") + result = parse_slash_command(text, "testbot") assert result is not None assert result.command == "help" From fe661268cbba2ddae920963627c5caf8ae2c8dd9 Mon Sep 17 00:00:00 2001 From: Sandro Date: Tue, 27 Jan 2026 23:36:14 +0000 Subject: [PATCH 3/4] Improved tests. --- CHANGELOG.md | 19 +- .../unit_tests/slash_commands/test_parser.py | 405 +++++++----------- 2 files changed, 153 insertions(+), 271 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9617b326..26f2b65f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,7 +44,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed -- Removed `/review` and `/security-audit` slash commands in favor of builtin skills. - Removed builtin `maintaining-changelog` skill in favor of the new changelog subagent - Removed `pull_request.branch_name_convention` from `.daiv.yml` configuration file. **BREAKING CHANGE**: Branch name convention must now be defined in the `AGENTS.md` file instead. - Removed Celery worker configuration and bootstrap scripts. @@ -64,7 +63,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added OpenRouter support to Anthropic caching middleware, reducing costs. - Added `FileNavigationMiddleware`, `FileEditingMiddleware`, `MergeRequestMiddleware` and `WebSearchMiddleware` in replacement of toolkits, leveraging LangChain v1 middlewares capabilities to inject the system prompt and tools into the model call. - Added `EXECUTION_THINKING_LEVEL` configuration to `PlanAndExecuteAgent` to allow users to enable thinking for execution tasks. -- Added `/clone-to-topic` slash command to clone issues to all repositories matching specified topics, enabling bulk distribution of issues across multiple repositories. +- Added `/clone-to-topic` quick action to clone issues to all repositories matching specified topics, enabling bulk distribution of issues across multiple repositories. ### Changed @@ -122,10 +121,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Migrated project from Python 3.13 to Python 3.14. - Refactored repository configuration file schema to be more flexible and easier to use. **BREAKING CHANGE** - Moved tools from `daiv/automation/tools` to `daiv/automation/agents/tools`. -- Moved slash commands from `daiv/automation/slash_commands` to `daiv/slash_commands`. -- Migrated slash command `help` to activate as `@daiv /help` instead of `@daiv help`. **BREAKING CHANGE** -- Migrated slash command `plan execute` to activate as `@daiv /approve-plan` instead of `@daiv plan execute`. **BREAKING CHANGE** -- Migrated slash command `plan revise` to activate as `@daiv /revise-plan` instead of `@daiv plan revise`. **BREAKING CHANGE** +- Moved quick actions from `daiv/automation/quick_actions` to `daiv/quick_actions`. +- Migrated quick action `help` to activate as `@daiv /help` instead of `@daiv help`. **BREAKING CHANGE** +- Migrated quick action `plan execute` to activate as `@daiv /approve-plan` instead of `@daiv plan execute`. **BREAKING CHANGE** +- Migrated quick action `plan revise` to activate as `@daiv /revise-plan` instead of `@daiv plan revise`. **BREAKING CHANGE** - Updated project dependencies. - Updated documentation. @@ -157,14 +156,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Added slash commands feature to allow users perform actions by commenting on the merge request or issue. -- Added slash commands to allow users to trigger plan revision by commenting `@daiv plan revise` on the issue. +- Added quick actions feature to allow users perform actions by commenting on the merge request or issue. +- Added quick actions to allow users to trigger plan revision by commenting `@daiv plan revise` on the issue. ### Changed - Migrated `RunSandboxCommandsTool` and `RunSandboxCodeTool` to be async only. -- Migrated `PipelineFixerAgent` to be triggered by a slash command instead of a webhook, allowing users to request a repair plan to fix pipelines by commenting `@daiv pipeline repair` on the merge request. -- Migrated `IssueAddressorAgent` plan approval to be triggered by a slash command, allowing users to request a plan approval by commenting `@daiv plan execute` on the issue. +- Migrated `PipelineFixerAgent` to be triggered by a quick action instead of a webhook, allowing users to request a repair plan to fix pipelines by commenting `@daiv pipeline repair` on the merge request. +- Migrated `IssueAddressorAgent` plan approval to be triggered by a quick action, allowing users to request a plan approval by commenting `@daiv plan execute` on the issue. ### Fixed diff --git a/tests/unit_tests/slash_commands/test_parser.py b/tests/unit_tests/slash_commands/test_parser.py index 94b849eb..84a89d9a 100644 --- a/tests/unit_tests/slash_commands/test_parser.py +++ b/tests/unit_tests/slash_commands/test_parser.py @@ -3,264 +3,147 @@ from slash_commands.parser import parse_slash_command -class TestParseSlashCommand: - def test_parse_simple_command(self): - """Test parsing a simple bot command.""" - note_body = "@testbot /help" - result = parse_slash_command(note_body, "testbot") - - assert result is not None - assert result.command == "help" - assert result.args == [] - assert result.raw == "@testbot /help" - - def test_parse_command_with_args(self): - """Test parsing a command with arguments.""" - note_body = "@testbot /assign user1 user2" - result = parse_slash_command(note_body, "testbot") - - assert result is not None - assert result.command == "assign" - assert result.args == ["user1", "user2"] - assert result.raw == "@testbot /assign user1 user2" - - def test_parse_command_with_quoted_args(self): - """Test parsing a command with quoted arguments.""" - note_body = '@testbot /create "test issue" --label "bug fix"' - result = parse_slash_command(note_body, "testbot") - - assert result is not None - assert result.command == "create" - assert result.args == ["test issue", "--label", "bug fix"] - assert result.raw == '@testbot /create "test issue" --label "bug fix"' - - def test_parse_case_insensitive_bot_name(self): - """Test that bot name matching is case insensitive.""" - note_body = "@TestBot /help" - result = parse_slash_command(note_body, "testbot") - - assert result is not None - assert result.command == "help" - assert result.raw == "@TestBot /help" - - def test_parse_case_insensitive_command(self): - """Test that command is converted to lowercase.""" - note_body = "@testbot /HELP" - result = parse_slash_command(note_body, "testbot") - - assert result is not None - assert result.command == "help" # Should be lowercase - - def test_parse_command_in_middle_of_text(self): - """Test parsing command that appears in middle of note.""" - note_body = "Some text before\n@testbot /help\nSome text after" - result = parse_slash_command(note_body, "testbot") - - assert result is not None - assert result.command == "help" - assert result.raw == "@testbot /help" - - def test_parse_first_command_only(self): - """Test that only first command is parsed when multiple exist.""" - note_body = "@testbot /help\n@testbot /status" - result = parse_slash_command(note_body, "testbot") - - assert result is not None - assert result.command == "help" - assert result.raw == "@testbot /help" - - def test_parse_no_command_found(self): - """Test when no bot command is found.""" - note_body = "Just some regular text without commands" - result = parse_slash_command(note_body, "testbot") - - assert result is None - - def test_parse_different_bot_name(self): - """Test that commands for different bots are ignored.""" - note_body = "@otherbot /help" - result = parse_slash_command(note_body, "testbot") - - assert result is None - - def test_parse_email_address_ignored(self): - """Test that email addresses are not matched as bot commands.""" - note_body = "Contact testbot@example.com for help" - result = parse_slash_command(note_body, "testbot") - - assert result is None - - def test_parse_partial_bot_name_ignored(self): - """Test that partial bot name matches are ignored.""" - note_body = "@testbotx /help" # Extra character - result = parse_slash_command(note_body, "testbot") - - assert result is None - - def test_parse_bot_name_with_special_chars(self): - """Test parsing bot name that contains special regex characters.""" - note_body = "@test.bot /help" - result = parse_slash_command(note_body, "test.bot") - - assert result is not None - assert result.command == "help" - - def test_parse_command_with_newline_in_middle(self): - """Test that commands stop at newlines.""" - note_body = "@testbot /help arg1\nthis should not be included" - result = parse_slash_command(note_body, "testbot") - - assert result is not None - assert result.command == "help" - assert result.args == ["arg1"] - assert "this should not be included" not in result.raw - - def test_parse_empty_command(self): - """Test parsing when bot is mentioned but no command follows.""" - note_body = "@testbot " # Just whitespace after mention - result = parse_slash_command(note_body, "testbot") - - assert result is None - - def test_parse_command_with_tabs(self): - """Test parsing command with tab characters.""" - note_body = "@testbot\t\t/help\targ1" - result = parse_slash_command(note_body, "testbot") - - assert result is not None - assert result.command == "help" - assert result.args == ["arg1"] - - def test_parse_command_with_complex_quoting(self): - """Test parsing command with complex shell-style quoting.""" - note_body = "@testbot /create 'single quotes' \"double quotes\" unquoted" - result = parse_slash_command(note_body, "testbot") - - assert result is not None - assert result.command == "create" - assert result.args == ["single quotes", "double quotes", "unquoted"] - - def test_parse_command_shlex_error_handling(self): - """Test handling of shlex parsing errors (unmatched quotes).""" - note_body = '@testbot /create "unmatched quote' - result = parse_slash_command(note_body, "testbot") - - assert result is None - - def test_parse_empty_note_body(self): - """Test parsing empty note body.""" - result = parse_slash_command("", "testbot") - assert result is None - - def test_parse_none_note_body(self): - """Test parsing None note body.""" - with pytest.raises(TypeError): - parse_slash_command(None, "testbot") # type: ignore - - def test_parse_whitespace_only_note_body(self): - """Test parsing note body with only whitespace.""" - result = parse_slash_command(" \n\t ", "testbot") - assert result is None - - def test_parse_bare_slash_command(self): - """Test parsing bare slash command (/command).""" - text = "/help" - result = parse_slash_command(text, "testbot") - - assert result is not None - assert result.command == "help" - assert result.args == [] - assert result.raw == "/help" - - def test_parse_bare_slash_command_with_args(self): - """Test parsing bare slash command with arguments.""" - text = "/review please check the security aspects" - result = parse_slash_command(text, "testbot") - - assert result is not None - assert result.command == "review" - assert result.args == ["please", "check", "the", "security", "aspects"] - assert result.raw == "/review please check the security aspects" - - def test_parse_bare_slash_command_with_leading_whitespace(self): - """Test parsing bare slash command with leading whitespace.""" - text = " /help" - result = parse_slash_command(text, "testbot") - - assert result is not None - assert result.command == "help" - assert result.args == [] - assert "/help" in result.raw - - def test_parse_bare_slash_command_in_multiline(self): - """Test parsing bare slash command in multiline text.""" - text = "Some text before\n/help\nSome text after" - result = parse_slash_command(text, "testbot") - - assert result is None - - def test_parse_prioritizes_mention_over_bare(self): - """Test that mention format is prioritized over bare format.""" - text = "/bare-command\n@testbot /mention-command" - result = parse_slash_command(text, "testbot") - - # Should find the mention format first - assert result is not None - assert result.command == "mention-command" - - def test_parse_bare_slash_command_with_quoted_args(self): - """Test parsing bare slash command with quoted arguments.""" - text = '/security-audit "check authentication"' - result = parse_slash_command(text, "testbot") - - assert result is not None - assert result.command == "security-audit" - assert result.args == ["check authentication"] - - def test_parse_bare_slash_at_start_of_line(self): - """Test parsing bare slash command at the start of a line.""" - text = "/help me understand" - result = parse_slash_command(text, "testbot") - - assert result is not None - assert result.command == "help" - assert result.args == ["me", "understand"] - - def test_parse_bare_slash_not_mid_word(self): - """Test that slash in middle of word is not detected.""" - text = "http://example.com/help" - result = parse_slash_command(text, "testbot") - - assert result is None - - def test_parse_bare_slash_command_case_insensitive(self): - """Test that bare slash commands are converted to lowercase.""" - text = "/HELP" - result = parse_slash_command(text, "testbot") - - assert result is not None - assert result.command == "help" - - def test_parse_bare_slash_shlex_error(self): - """Test handling of shlex parsing errors in bare format.""" - text = '/command "unmatched quote' - result = parse_slash_command(text, "testbot") - - assert result is None - - def test_parse_bare_slash_only(self): - """Test parsing when only slash is present.""" - text = "/" - result = parse_slash_command(text, "testbot") - - # Should return None as there's no command after the slash - assert result is None - - def test_parse_bare_slash_with_tabs(self): - """Test parsing bare slash command with tabs.""" - text = "/help\targ1\targ2" - result = parse_slash_command(text, "testbot") - - assert result is not None - assert result.command == "help" - assert result.args == ["arg1", "arg2"] +def _assert_parsed(note_body: str, bot_name: str, *, command: str, args: list[str], raw: str) -> None: + result = parse_slash_command(note_body, bot_name) + + assert result is not None + assert result.command == command + assert result.args == args + assert result.raw == raw + + +def _assert_not_parsed(note_body: str, bot_name: str) -> None: + assert parse_slash_command(note_body, bot_name) is None + + +MENTION_CASES = [ + pytest.param("@testbot /help", "testbot", "help", [], "@testbot /help", id="simple"), + pytest.param( + "@testbot /assign user1 user2", + "testbot", + "assign", + ["user1", "user2"], + "@testbot /assign user1 user2", + id="args", + ), + pytest.param( + '@testbot /create "test issue" --label "bug fix"', + "testbot", + "create", + ["test issue", "--label", "bug fix"], + '@testbot /create "test issue" --label "bug fix"', + id="quoted", + ), + pytest.param("@TestBot /help", "testbot", "help", [], "@TestBot /help", id="case-insensitive-bot"), + pytest.param("@testbot /HELP", "testbot", "help", [], "@testbot /HELP", id="case-insensitive-command"), + pytest.param( + "Some text before\n@testbot /help\nSome text after", + "testbot", + "help", + [], + "@testbot /help", + id="middle-of-text", + ), + pytest.param("@testbot /help\n@testbot /status", "testbot", "help", [], "@testbot /help", id="first-command-only"), + pytest.param("@test.bot /help", "test.bot", "help", [], "@test.bot /help", id="special-chars-bot"), + pytest.param( + "@testbot /help arg1\nthis should not be included", + "testbot", + "help", + ["arg1"], + "@testbot /help arg1", + id="newline-stops", + ), + pytest.param("@testbot\t\t/help\targ1", "testbot", "help", ["arg1"], "@testbot\t\t/help\targ1", id="tabs"), + pytest.param( + "@testbot /create 'single quotes' \"double quotes\" unquoted", + "testbot", + "create", + ["single quotes", "double quotes", "unquoted"], + "@testbot /create 'single quotes' \"double quotes\" unquoted", + id="complex-quoting", + ), +] + +BARE_CASES = [ + pytest.param("/help", "testbot", "help", [], "/help", id="bare-simple"), + pytest.param( + "/review please check the security aspects", + "testbot", + "review", + ["please", "check", "the", "security", "aspects"], + "/review please check the security aspects", + id="bare-args", + ), + pytest.param(" /help", "testbot", "help", [], "/help", id="bare-leading-whitespace"), + pytest.param( + '/security-audit "check authentication"', + "testbot", + "security-audit", + ["check authentication"], + '/security-audit "check authentication"', + id="bare-quoted", + ), + pytest.param( + "/help me understand", "testbot", "help", ["me", "understand"], "/help me understand", id="bare-start-line" + ), + pytest.param("/HELP", "testbot", "help", [], "/HELP", id="bare-case-insensitive"), + pytest.param("/help\targ1\targ2", "testbot", "help", ["arg1", "arg2"], "/help\targ1\targ2", id="bare-tabs"), +] + +INVALID_CASES = [ + pytest.param("Just some regular text without commands", "testbot", id="no-command"), + pytest.param("@otherbot /help", "testbot", id="different-bot"), + pytest.param("Contact testbot@example.com for help", "testbot", id="email"), + pytest.param("@testbotx /help", "testbot", id="partial-bot"), + pytest.param("@testbot ", "testbot", id="mention-no-command"), + pytest.param("", "testbot", id="empty-note"), + pytest.param(" \n\t ", "testbot", id="whitespace-only"), + pytest.param("http://example.com/help", "testbot", id="slash-mid-word"), + pytest.param("Some text before\n/help\nSome text after", "testbot", id="bare-multiline"), + pytest.param("/", "testbot", id="bare-slash-only"), +] + +INVALID_SHLEX_CASES = [ + pytest.param('@testbot /create "unmatched quote', "testbot", id="mention-unmatched-quote"), + pytest.param('/command "unmatched quote', "testbot", id="bare-unmatched-quote"), +] + + +@pytest.mark.parametrize("note_body, bot_name, command, args, raw", MENTION_CASES) +def test_parse_mention_command_cases(note_body: str, bot_name: str, command: str, args: list[str], raw: str) -> None: + """Test parsing mention-based slash commands.""" + _assert_parsed(note_body, bot_name, command=command, args=args, raw=raw) + + +@pytest.mark.parametrize("note_body, bot_name, command, args, raw", BARE_CASES) +def test_parse_bare_command_cases(note_body: str, bot_name: str, command: str, args: list[str], raw: str) -> None: + """Test parsing bare slash commands.""" + _assert_parsed(note_body, bot_name, command=command, args=args, raw=raw) + + +@pytest.mark.parametrize("note_body, bot_name", INVALID_CASES) +def test_parse_invalid_cases(note_body: str, bot_name: str) -> None: + """Test inputs that should not parse into commands.""" + _assert_not_parsed(note_body, bot_name) + + +@pytest.mark.parametrize("note_body, bot_name", INVALID_SHLEX_CASES) +def test_parse_shlex_error_handling(note_body: str, bot_name: str) -> None: + """Test handling of shlex parsing errors (unmatched quotes).""" + _assert_not_parsed(note_body, bot_name) + + +def test_parse_prioritizes_mention_over_bare() -> None: + """Test that mention format is prioritized over bare format.""" + text = "/bare-command\n@testbot /mention-command" + result = parse_slash_command(text, "testbot") + + assert result is not None + assert result.command == "mention-command" + + +def test_parse_none_note_body() -> None: + """Test parsing None note body.""" + with pytest.raises(TypeError): + parse_slash_command(None, "testbot") # type: ignore From 115e9d96d2078245124545934690a0d5b377348e Mon Sep 17 00:00:00 2001 From: Sandro Date: Tue, 27 Jan 2026 23:58:44 +0000 Subject: [PATCH 4/4] Removed file that was accidently commited. --- Provide a code review for the given merg.md | 84 --------------------- 1 file changed, 84 deletions(-) delete mode 100644 Provide a code review for the given merg.md diff --git a/Provide a code review for the given merg.md b/Provide a code review for the given merg.md deleted file mode 100644 index 7001ee01..00000000 --- a/Provide a code review for the given merg.md +++ /dev/null @@ -1,84 +0,0 @@ -Provide a code review for the given merge request. - -**Agent assumptions (applies to all agents and subagents):** -- All tools are functional and will work without error. Do not test tools or make exploratory calls. Make sure this is clear to every subagent that is launched. -- Only call a tool if it is required to complete the task. Every tool call should have a clear purpose. - -To do this, follow these steps precisely: - -1. Launch a general purpose agent to check if any of the following are true: - - The merge request is closed - - The merge request is a draft - - The merge request does not need code review (e.g. automated PR, trivial change that is obviously correct) - - If any condition is true, stop and do not proceed. - -Note: Still review DAIV generated MR's. - -2. Launch a general purpose agent to view the merge request and return a summary of the changes - -3. Launch 3 agents in parallel to independently review the changes. Each agent should return the list of issues, where each issue includes a description and the reason it was flagged (e.g. "AGENTS.md adherence", "bug"). The agents should do the following: - - Agent 1: AGENTS.md compliance agent - Audit changes for AGENTS.md compliance. - - Agent 2: General purpose bug agent (parallel subagent with agent 3) - Scan for obvious bugs. Focus only on the diff itself without reading extra context. Flag only significant bugs; ignore nitpicks and likely false positives. Do not flag issues that you cannot validate without looking at context outside of the git diff. - - Agent 3: General purpose bug agent (parallel subagent with agent 2) - Look for problems that exist in the introduced code. This could be security issues, incorrect logic, etc. Only look for issues that fall within the changed code. - - **CRITICAL: We only want HIGH SIGNAL issues.** Flag issues where: - - The code will fail to compile or parse (syntax errors, type errors, missing imports, unresolved references) - - The code will definitely produce wrong results regardless of inputs (clear logic errors) - - Clear, unambiguous AGENTS.md violations where you can quote the exact rule being broken - - Do NOT flag: - - Code style or quality concerns - - Potential issues that depend on specific inputs or state - - Subjective suggestions or improvements - - If you are not certain an issue is real, do not flag it. False positives erode trust and waste reviewer time. - - In addition to the above, each subagent should be told the PR ID, title and description. This will help provide context regarding the author's intent. - -4. For each issue found in the previous step by agents 2 and 3, launch parallel subagents to validate the issue. These subagents should get the PR ID, title and description along with a description of the issue. The agent's job is to review the issue to validate that the stated issue is truly an issue with high confidence. For example, if an issue such as "variable is not defined" was flagged, the subagent's job would be to validate that is actually true in the code. Another example would be AGENTS.md issues. The agent should validate that the AGENTS.md rule that was violated is scoped for this file and is actually violated. Use subagents for bugs, logic issues, and AGENTS.md violations. - -5. Filter out any issues that were not validated in step 4. This step will give us our list of high signal issues for our review. - -6. If issues were found, skip to step 7 to reply. - - If NO issues were found, reply with the following message: - "No issues found. Checked for bugs and AGENTS.md compliance." - -7. Create a list of all comments that you plan on leaving. This is only for you to make sure you are comfortable with the comments. Do not post this list anywhere. - -8. Post inline comments for each issue using `gitlab` tool with `project-merge-request-draft-note` providing a `position` argument. For each comment: - - Provide a brief description of the issue - - For small, self-contained fixes, include a committable suggestion block - - For larger fixes (6+ lines, structural changes, or changes spanning multiple locations), describe the issue and suggested fix without a suggestion block - - Never post a committable suggestion UNLESS committing the suggestion fixes the issue entirely. If follow up steps are required, do not leave a committable suggestion. - - **IMPORTANT: Only post ONE comment per unique issue. Do not post duplicate comments.** - -Use this list when evaluating issues in Steps 4 and 5 (these are false positives, do NOT flag): - -- Pre-existing issues -- Something that appears to be a bug but is actually correct -- Pedantic nitpicks that a senior engineer would not flag -- Issues that a linter will catch (do not run the linter to verify) -- General code quality concerns (e.g., lack of test coverage, general security issues) unless explicitly required in AGENTS.md -- Issues mentioned in AGENTS.md but explicitly silenced in the code (e.g., via a lint ignore comment) - -Notes: - -- Use `gitlab` tool to interact with GitLab (e.g., fetch merge requests, create inline comments). Do not use web fetch. -- Create a todo list before starting. -- You must cite and link each issue in inline comments (e.g., if referring to a AGENTS.md, include a link to it). -- When linking to code in inline comments, follow the following format precisely, otherwise the Markdown preview won't render correctly: http://gitlab:8929/anthropics/claude-code/-/blob/c21d3c10bc8e898b7ac1a2d745bdc9bc4e423afe/package.json#L10-L15 - - Requires full git sha - - You must provide the full sha. Commands like `http://gitlab:8929/owner/repo/-/blob/$(git rev-parse HEAD)/foo/bar` will not work, since your comment will be directly rendered in Markdown. - - Repo name must match the repo you're code reviewing - - # sign after the file name - - Line range format is L[start]-L[end] - - Provide at least 1 line of context before and after, centered on the line you are commenting about (eg. if you are commenting about lines 5-6, you should link to `L4-7`)