diff --git a/.github/workflows/droid-review.yml b/.github/workflows/droid-review.yml index 051fff7..04ac059 100644 --- a/.github/workflows/droid-review.yml +++ b/.github/workflows/droid-review.yml @@ -5,7 +5,7 @@ on: types: [opened, ready_for_review, reopened] jobs: - droid-review: + prepare: if: github.event.pull_request.draft == false runs-on: ubuntu-latest permissions: @@ -13,15 +13,125 @@ jobs: pull-requests: write issues: write id-token: write - actions: read + outputs: + comment_id: ${{ steps.prepare.outputs.comment_id }} + run_code_review: ${{ steps.prepare.outputs.run_code_review }} + run_security_review: ${{ steps.prepare.outputs.run_security_review }} steps: - name: Checkout repository uses: actions/checkout@v5 with: fetch-depth: 1 - - name: Run Droid Auto Review - uses: Factory-AI/droid-action@v1 + - name: Prepare + id: prepare + uses: Factory-AI/droid-action/prepare@v1 with: factory_api_key: ${{ secrets.FACTORY_API_KEY }} automatic_review: true + automatic_security_review: true + + code-review: + needs: prepare + if: needs.prepare.outputs.run_code_review == 'true' + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + issues: write + id-token: write + actions: read + steps: + - name: Checkout repository + uses: actions/checkout@v5 + with: + fetch-depth: 1 + + - name: Run Code Review + uses: Factory-AI/droid-action/review@v1 + with: + factory_api_key: ${{ secrets.FACTORY_API_KEY }} + tracking_comment_id: ${{ needs.prepare.outputs.comment_id }} + output_file: ${{ runner.temp }}/code-review-results.json + + - name: Upload Results + uses: actions/upload-artifact@v4 + with: + name: code-review-results + path: ${{ runner.temp }}/code-review-results.json + if-no-files-found: ignore + + security-review: + needs: prepare + if: needs.prepare.outputs.run_security_review == 'true' + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + issues: write + id-token: write + actions: read + steps: + - name: Checkout repository + uses: actions/checkout@v5 + with: + fetch-depth: 1 + + - name: Run Security Review + uses: Factory-AI/droid-action/security@v1 + with: + factory_api_key: ${{ secrets.FACTORY_API_KEY }} + tracking_comment_id: ${{ needs.prepare.outputs.comment_id }} + security_severity_threshold: medium + output_file: ${{ runner.temp }}/security-results.json + + - name: Upload Results + uses: actions/upload-artifact@v4 + with: + name: security-results + path: ${{ runner.temp }}/security-results.json + if-no-files-found: ignore + + combine: + needs: [prepare, code-review, security-review] + # Only run combine when BOTH code review AND security review were executed + if: | + always() && + needs.prepare.outputs.run_code_review == 'true' && + needs.prepare.outputs.run_security_review == 'true' + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + issues: write + id-token: write + actions: read + steps: + - name: Checkout repository + uses: actions/checkout@v5 + with: + fetch-depth: 1 + + - name: Download Code Review Results + uses: actions/download-artifact@v4 + with: + name: code-review-results + path: ${{ runner.temp }} + continue-on-error: true + + - name: Download Security Results + uses: actions/download-artifact@v4 + with: + name: security-results + path: ${{ runner.temp }} + continue-on-error: true + + - name: Combine Results + uses: Factory-AI/droid-action/combine@v1 + with: + factory_api_key: ${{ secrets.FACTORY_API_KEY }} + tracking_comment_id: ${{ needs.prepare.outputs.comment_id }} + code_review_results: ${{ runner.temp }}/code-review-results.json + security_results: ${{ runner.temp }}/security-results.json + code_review_status: ${{ needs.code-review.result }} + security_review_status: ${{ needs.security-review.result }} diff --git a/README.md b/README.md index b3cfb4d..be17208 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,8 @@ This GitHub Action powers the Factory **Droid** app. It watches your pull requests for the two supported commands and runs a full Droid Exec session to help you ship faster: -* `@droid fill` — turns a bare pull request into a polished description that matches your template or our opinionated fallback. -* `@droid review` — performs an automated code review, surfaces potential bugs, and leaves inline comments directly on the diff. +- `@droid fill` — turns a bare pull request into a polished description that matches your template or our opinionated fallback. +- `@droid review` — performs an automated code review, surfaces potential bugs, and leaves inline comments directly on the diff. Everything runs inside GitHub Actions using your Factory API key, so the bot never leaves your repository and operates with the permissions you grant. @@ -18,11 +18,11 @@ Everything runs inside GitHub Actions using your Factory API key, so the bot nev ## Installation 1. **Install the Droid GitHub App** - * Install from the Factory dashboard and grant it access to the repositories where you want Droid to operate. + - Install from the Factory dashboard and grant it access to the repositories where you want Droid to operate. 2. **Create a Factory API Key** - * Generate a token at [https://app.factory.ai/settings/api-keys](https://app.factory.ai/settings/api-keys) and save it as `FACTORY_API_KEY` in your repository or organization secrets. + - Generate a token at [https://app.factory.ai/settings/api-keys](https://app.factory.ai/settings/api-keys) and save it as `FACTORY_API_KEY` in your repository or organization secrets. 3. **Add the Action Workflows** - * Create two workflow files under `.github/workflows/` to separate on-demand tagging from automatic PR reviews. + - Create two workflow files under `.github/workflows/` to separate on-demand tagging from automatic PR reviews. `droid.yml` (responds to explicit `@droid` mentions): @@ -105,26 +105,28 @@ Once committed, tagging `@droid fill` or `@droid review` on an open PR will trig ## Using the Commands ### `@droid fill` -* Place the command in the PR description or in a top-level comment. -* Droid searches for common PR template locations (`.github/pull_request_template.md`, etc.). When a template exists, it fills the sections; otherwise it writes a structured summary (overview, changes, testing, rollout). -* The original request is replaced with the generated description so reviewers can merge immediately. + +- Place the command in the PR description or in a top-level comment. +- Droid searches for common PR template locations (`.github/pull_request_template.md`, etc.). When a template exists, it fills the sections; otherwise it writes a structured summary (overview, changes, testing, rollout). +- The original request is replaced with the generated description so reviewers can merge immediately. ### `@droid review` -* Mention `@droid review` in a PR comment. -* Droid inspects the diff, prioritizes potential bugs or high-impact issues, and leaves inline comments directly on the changed lines. -* A short summary comment is posted in the original thread highlighting the findings and linking to any inline feedback. + +- Mention `@droid review` in a PR comment. +- Droid inspects the diff, prioritizes potential bugs or high-impact issues, and leaves inline comments directly on the changed lines. +- A short summary comment is posted in the original thread highlighting the findings and linking to any inline feedback. ## Configuration Essentials -| Input | Purpose | -| --- | --- | -| `factory_api_key` | **Required.** Grants Droid Exec permission to run via Factory. | -| `github_token` | Optional override if you prefer a custom GitHub App/token. By default the installed app token is used. | -| `review_model` | Optional. Override the model used for code review (e.g., `claude-sonnet-4-5-20250929`, `gpt-5.1-codex`). Only applies to review flows. | -| `fill_model` | Optional. Override the model used for PR description fill (e.g., `claude-sonnet-4-5-20250929`, `gpt-5.1-codex`). Only applies to fill flows. | +| Input | Purpose | +| ----------------- | -------------------------------------------------------------------------------------------------------------------------------------------- | +| `factory_api_key` | **Required.** Grants Droid Exec permission to run via Factory. | +| `github_token` | Optional override if you prefer a custom GitHub App/token. By default the installed app token is used. | +| `review_model` | Optional. Override the model used for code review (e.g., `claude-sonnet-4-5-20250929`, `gpt-5.1-codex`). Only applies to review flows. | +| `fill_model` | Optional. Override the model used for PR description fill (e.g., `claude-sonnet-4-5-20250929`, `gpt-5.1-codex`). Only applies to fill flows. | ## Troubleshooting & Support -* Check the workflow run linked from the Droid tracking comment for execution logs. -* Verify that the workflow file and repository allow the GitHub App to run (branch protections can block bots). -* Need more detail? Start with the [Setup Guide](./docs/setup.md) or [FAQ](./docs/faq.md). +- Check the workflow run linked from the Droid tracking comment for execution logs. +- Verify that the workflow file and repository allow the GitHub App to run (branch protections can block bots). +- Need more detail? Start with the [Setup Guide](./docs/setup.md) or [FAQ](./docs/faq.md). diff --git a/action.yml b/action.yml index f05b5fe..89ba519 100644 --- a/action.yml +++ b/action.yml @@ -59,6 +59,38 @@ inputs: description: "Automatically run the review flow for pull request contexts without requiring an explicit @droid review command. Only supported for PR-related events." required: false default: "false" + automatic_security_review: + description: "Automatically run the security review flow for pull request contexts without requiring an explicit @droid security-review command. Only supported for PR-related events." + required: false + default: "false" + security_model: + description: "Override the model used for security review (e.g., 'claude-sonnet-4-5-20250929', 'gpt-5.1-codex'). Only applies to security review flows." + required: false + default: "" + security_severity_threshold: + description: "Minimum severity to report in security reviews (critical, high, medium, low). Findings below this threshold will be filtered out." + required: false + default: "medium" + security_block_on_critical: + description: "Submit REQUEST_CHANGES review when critical severity findings are detected." + required: false + default: "true" + security_block_on_high: + description: "Submit REQUEST_CHANGES review when high severity findings are detected." + required: false + default: "false" + security_notify_team: + description: "GitHub team to @mention on critical findings (e.g., '@org/security-team')." + required: false + default: "" + security_scan_schedule: + description: "Enable scheduled security scans. Set to 'true' for schedule events to trigger full repository scans." + required: false + default: "false" + security_scan_days: + description: "Number of days of commits to scan for scheduled security scans. Only applies when security_scan_schedule is enabled." + required: false + default: "7" review_model: description: "Override the model used for code review (e.g., 'claude-sonnet-4-5-20250929', 'gpt-5.1-codex'). Only applies to review flows." required: false @@ -135,6 +167,14 @@ runs: DEFAULT_WORKFLOW_TOKEN: ${{ github.token }} TRACK_PROGRESS: ${{ inputs.track_progress }} AUTOMATIC_REVIEW: ${{ inputs.automatic_review }} + AUTOMATIC_SECURITY_REVIEW: ${{ inputs.automatic_security_review }} + SECURITY_MODEL: ${{ inputs.security_model }} + SECURITY_SEVERITY_THRESHOLD: ${{ inputs.security_severity_threshold }} + SECURITY_BLOCK_ON_CRITICAL: ${{ inputs.security_block_on_critical }} + SECURITY_BLOCK_ON_HIGH: ${{ inputs.security_block_on_high }} + SECURITY_NOTIFY_TEAM: ${{ inputs.security_notify_team }} + SECURITY_SCAN_SCHEDULE: ${{ inputs.security_scan_schedule }} + SECURITY_SCAN_DAYS: ${{ inputs.security_scan_days }} REVIEW_MODEL: ${{ inputs.review_model }} REASONING_EFFORT: ${{ inputs.reasoning_effort }} FILL_MODEL: ${{ inputs.fill_model }} @@ -178,6 +218,67 @@ runs: env: EXPERIMENTAL_ALLOWED_DOMAINS: ${{ inputs.experimental_allowed_domains }} + - name: Install Security Skills + if: steps.prepare.outputs.contains_trigger == 'true' && steps.prepare.outputs.install_security_skills == 'true' + shell: bash + run: | + echo "Installing security skills from Factory-AI/skills..." + SKILLS_DIR="$HOME/.factory/skills" + mkdir -p "$SKILLS_DIR" + + # Clone public skills repo (sparse checkout for efficiency) + TEMP_DIR=$(mktemp -d) + git clone --filter=blob:none --sparse \ + "https://github.com/Factory-AI/skills.git" \ + "$TEMP_DIR" 2>/dev/null || { + echo "Warning: Could not clone skills repo. Security skills will not be available." + exit 0 + } + + cd "$TEMP_DIR" + git sparse-checkout set \ + skills/threat-model-generation \ + skills/commit-security-scan \ + skills/vulnerability-validation \ + skills/security-review 2>/dev/null || true + + # Copy skills to ~/.factory/skills/ and track installed count + INSTALLED_COUNT=0 + for skill in threat-model-generation commit-security-scan vulnerability-validation security-review; do + if [ -d "skills/$skill" ]; then + cp -r "skills/$skill" "$SKILLS_DIR/" + echo " Installed skill: $skill" + INSTALLED_COUNT=$((INSTALLED_COUNT + 1)) + else + echo " Warning: Skill not found in repo: $skill" + fi + done + + # Cleanup + rm -rf "$TEMP_DIR" + + # Verify at least one skill was installed + if [ "$INSTALLED_COUNT" -eq 0 ]; then + echo "Warning: No security skills were installed. The skills may not exist in the Factory-AI/skills repository." + echo "Security review will proceed but may have limited functionality." + else + echo "Security skills installation complete ($INSTALLED_COUNT skills installed)" + fi + + # Verify skills exist in the target directory + echo "Verifying installed skills in $SKILLS_DIR..." + VERIFIED_COUNT=0 + for skill in threat-model-generation commit-security-scan vulnerability-validation security-review; do + if [ -d "$SKILLS_DIR/$skill" ]; then + echo " Verified: $skill" + VERIFIED_COUNT=$((VERIFIED_COUNT + 1)) + fi + done + + if [ "$VERIFIED_COUNT" -ne "$INSTALLED_COUNT" ]; then + echo "Warning: Skill verification mismatch. Expected $INSTALLED_COUNT, found $VERIFIED_COUNT in $SKILLS_DIR" + fi + - name: Checkout PR branch for review if: steps.prepare.outputs.contains_trigger == 'true' && steps.prepare.outputs.review_pr_number != '' shell: bash diff --git a/base-action/src/index.ts b/base-action/src/index.ts index 2623880..860884a 100644 --- a/base-action/src/index.ts +++ b/base-action/src/index.ts @@ -29,8 +29,7 @@ async function run() { mcpTools: process.env.INPUT_MCP_TOOLS, systemPrompt: process.env.INPUT_SYSTEM_PROMPT, appendSystemPrompt: process.env.INPUT_APPEND_SYSTEM_PROMPT, - pathToDroidExecutable: - process.env.INPUT_PATH_TO_DROID_EXECUTABLE, + pathToDroidExecutable: process.env.INPUT_PATH_TO_DROID_EXECUTABLE, showFullOutput: process.env.INPUT_SHOW_FULL_OUTPUT, }); } catch (error) { diff --git a/base-action/src/run-droid.ts b/base-action/src/run-droid.ts index 3088cf8..015dc98 100644 --- a/base-action/src/run-droid.ts +++ b/base-action/src/run-droid.ts @@ -127,10 +127,12 @@ export async function runDroid(promptPath: string, options: DroidOptions) { const cfg = JSON.parse(options.mcpTools); const servers = cfg?.mcpServers || {}; const serverNames = Object.keys(servers); - + if (serverNames.length > 0) { - console.log(`Registering ${serverNames.length} MCP servers: ${serverNames.join(", ")}`); - + console.log( + `Registering ${serverNames.length} MCP servers: ${serverNames.join(", ")}`, + ); + for (const [name, def] of Object.entries(servers)) { const cmd = [def.command, ...(def.args || [])] .filter(Boolean) @@ -149,12 +151,15 @@ export async function runDroid(promptPath: string, options: DroidOptions) { .join(" "); const addCmd = `droid mcp add ${name} "${cmd}" ${envFlags}`.trim(); - + try { await execAsync(addCmd, { env: { ...process.env } }); console.log(` ✓ Registered MCP server: ${name}`); } catch (e: any) { - console.error(` ✗ Failed to register MCP server ${name}:`, e.message); + console.error( + ` ✗ Failed to register MCP server ${name}:`, + e.message, + ); throw e; } } @@ -190,15 +195,19 @@ export async function runDroid(promptPath: string, options: DroidOptions) { // Log custom arguments if any if (options.droidArgs && options.droidArgs.trim() !== "") { console.log(`Custom Droid arguments: ${options.droidArgs}`); - + // Check for deprecated MCP tool naming - const enabledToolsMatch = options.droidArgs.match(/--enabled-tools\s+["\']?([^"\']+)["\']?/); + const enabledToolsMatch = options.droidArgs.match( + /--enabled-tools\s+["\']?([^"\']+)["\']?/, + ); if (enabledToolsMatch && enabledToolsMatch[1]) { - const tools = enabledToolsMatch[1].split(",").map(t => t.trim()); - const oldStyleTools = tools.filter(t => t.startsWith("mcp__")); - + const tools = enabledToolsMatch[1].split(",").map((t) => t.trim()); + const oldStyleTools = tools.filter((t) => t.startsWith("mcp__")); + if (oldStyleTools.length > 0) { - console.warn(`Warning: Found ${oldStyleTools.length} tools with deprecated mcp__ prefix. Update to new pattern (e.g., github_comment___update_droid_comment)`); + console.warn( + `Warning: Found ${oldStyleTools.length} tools with deprecated mcp__ prefix. Update to new pattern (e.g., github_comment___update_droid_comment)`, + ); } } } @@ -253,7 +262,10 @@ export async function runDroid(promptPath: string, options: DroidOptions) { const parsed = JSON.parse(line); if (!sessionId && typeof parsed === "object" && parsed !== null) { const detectedSessionId = parsed.session_id; - if (typeof detectedSessionId === "string" && detectedSessionId.trim()) { + if ( + typeof detectedSessionId === "string" && + detectedSessionId.trim() + ) { sessionId = detectedSessionId; console.log(`Detected Droid session: ${sessionId}`); } @@ -278,7 +290,6 @@ export async function runDroid(promptPath: string, options: DroidOptions) { // In non-full-output mode, suppress non-JSON output } }); - }); // Handle stdout errors diff --git a/base-action/test/parse-shell-args.test.ts b/base-action/test/parse-shell-args.test.ts index b6f0dc4..297d5e3 100644 --- a/base-action/test/parse-shell-args.test.ts +++ b/base-action/test/parse-shell-args.test.ts @@ -8,14 +8,8 @@ describe("shell-quote parseShellArgs", () => { }); test("should parse simple arguments", () => { - expect(parseShellArgs("--auto medium")).toEqual([ - "--auto", - "medium", - ]); - expect(parseShellArgs("-s session-123")).toEqual([ - "-s", - "session-123", - ]); + expect(parseShellArgs("--auto medium")).toEqual(["--auto", "medium"]); + expect(parseShellArgs("-s session-123")).toEqual(["-s", "session-123"]); }); test("should handle double quotes", () => { @@ -27,10 +21,11 @@ describe("shell-quote parseShellArgs", () => { }); test("should handle single quotes", () => { - expect(parseShellArgs("--file '/tmp/prompt.md'")) - .toEqual(["--file", "/tmp/prompt.md"]); - expect(parseShellArgs("'arg with spaces'")) - .toEqual(["arg with spaces"]); + expect(parseShellArgs("--file '/tmp/prompt.md'")).toEqual([ + "--file", + "/tmp/prompt.md", + ]); + expect(parseShellArgs("'arg with spaces'")).toEqual(["arg with spaces"]); }); test("should handle escaped characters", () => { expect(parseShellArgs("arg\\ with\\ spaces")).toEqual(["arg with spaces"]); diff --git a/base-action/test/run-droid-mcp.test.ts b/base-action/test/run-droid-mcp.test.ts index ea652a2..98cde42 100644 --- a/base-action/test/run-droid-mcp.test.ts +++ b/base-action/test/run-droid-mcp.test.ts @@ -74,11 +74,15 @@ const mockSpawn = mock( mock.module("child_process", () => ({ exec: ( command: string, - options?: Record | ((err: Error | null, result?: any) => void), + options?: + | Record + | ((err: Error | null, result?: any) => void), maybeCallback?: (err: Error | null, result?: any) => void, ) => { const callback = - typeof options === "function" ? options : maybeCallback ?? (() => undefined); + typeof options === "function" + ? options + : (maybeCallback ?? (() => undefined)); setImmediate(async () => { try { @@ -98,7 +102,7 @@ let runDroid: RunDroidModule["runDroid"]; beforeAll(async () => { const module = (await import( - `../src/run-droid?mcp-test=${Math.random().toString(36).slice(2)}`, + `../src/run-droid?mcp-test=${Math.random().toString(36).slice(2)}` )) as RunDroidModule; prepareRunConfig = module.prepareRunConfig; runDroid = module.runDroid; @@ -139,23 +143,23 @@ describe("MCP Server Registration", () => { env: { GITHUB_TOKEN: "test-token", REPO_OWNER: "owner", - REPO_NAME: "repo" - } + REPO_NAME: "repo", + }, }, github_ci: { command: "bun", args: ["run", "/path/to/github-actions-server.ts"], env: { GITHUB_TOKEN: "test-token", - PR_NUMBER: "123" - } - } - } + PR_NUMBER: "123", + }, + }, + }, }); const options: DroidOptions = { mcpTools, - pathToDroidExecutable: "droid" + pathToDroidExecutable: "droid", }; const promptPath = await createPromptFile(); const tempDir = process.env.RUNNER_TEMP!; @@ -180,14 +184,14 @@ describe("MCP Server Registration", () => { mcpTools: "", }; const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - + expect(prepared.droidArgs).not.toContain("--mcp-config"); }); test("should handle invalid JSON in MCP config", () => { const options: DroidOptions = { mcpTools: "{ invalid json }", - pathToDroidExecutable: "droid" + pathToDroidExecutable: "droid", }; // prepareRunConfig doesn't parse MCP config, so it won't throw @@ -205,7 +209,8 @@ describe("MCP Server Registration", () => { console.warn = warnSpy as unknown as typeof console.warn; const options: DroidOptions = { - droidArgs: '--enabled-tools "mcp__github_comment__update_droid_comment,Execute"' + droidArgs: + '--enabled-tools "mcp__github_comment__update_droid_comment,Execute"', }; const promptPath = await createPromptFile(); @@ -216,8 +221,10 @@ describe("MCP Server Registration", () => { const warningMessages = warnSpy.mock.calls.map((args) => args[0]); expect( - warningMessages.some((msg) => - typeof msg === "string" && msg.includes("deprecated mcp__ prefix"), + warningMessages.some( + (msg) => + typeof msg === "string" && + msg.includes("deprecated mcp__ prefix"), ), ).toBe(true); } finally { @@ -232,7 +239,8 @@ describe("MCP Server Registration", () => { console.warn = warnSpy as unknown as typeof console.warn; const options: DroidOptions = { - droidArgs: '--enabled-tools "github_comment___update_droid_comment,Execute"' + droidArgs: + '--enabled-tools "github_comment___update_droid_comment,Execute"', }; const promptPath = await createPromptFile(); @@ -249,14 +257,17 @@ describe("MCP Server Registration", () => { test("should detect MCP tools with triple underscore pattern", () => { const options: DroidOptions = { - droidArgs: '--enabled-tools "github_ci___get_ci_status,github_comment___update_droid_comment"' + droidArgs: + '--enabled-tools "github_ci___get_ci_status,github_comment___update_droid_comment"', }; const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); - + // The args should be passed through correctly expect(prepared.droidArgs).toContain("--enabled-tools"); - expect(prepared.droidArgs).toContain("github_ci___get_ci_status,github_comment___update_droid_comment"); + expect(prepared.droidArgs).toContain( + "github_ci___get_ci_status,github_comment___update_droid_comment", + ); }); }); @@ -267,14 +278,14 @@ describe("MCP Server Registration", () => { failing_server: { command: "nonexistent", args: ["command"], - env: {} - } - } + env: {}, + }, + }, }); const options: DroidOptions = { mcpTools, - pathToDroidExecutable: "droid" + pathToDroidExecutable: "droid", }; const promptPath = await createPromptFile(); const tempDir = process.env.RUNNER_TEMP!; @@ -299,18 +310,18 @@ describe("MCP Server Registration", () => { describe("Environment Variables", () => { test("should include GITHUB_ACTION_INPUTS when present", () => { process.env.INPUT_ACTION_INPUTS_PRESENT = "true"; - + const options: DroidOptions = {}; const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); expect(prepared.env.GITHUB_ACTION_INPUTS).toBe("true"); - + delete process.env.INPUT_ACTION_INPUTS_PRESENT; }); test("should not include GITHUB_ACTION_INPUTS when not present", () => { delete process.env.INPUT_ACTION_INPUTS_PRESENT; - + const options: DroidOptions = {}; const prepared = prepareRunConfig("/tmp/test-prompt.txt", options); diff --git a/base-action/test/run-droid.test.ts b/base-action/test/run-droid.test.ts index 4bb95d2..36f7cb6 100644 --- a/base-action/test/run-droid.test.ts +++ b/base-action/test/run-droid.test.ts @@ -43,7 +43,7 @@ describe("prepareRunConfig", () => { "exec", "--output-format", "stream-json", -"--skip-permissions-unsafe", + "--skip-permissions-unsafe", "--max-turns", "10", "--model", @@ -63,7 +63,7 @@ describe("prepareRunConfig", () => { "exec", "--output-format", "stream-json", - "--skip-permissions-unsafe", + "--skip-permissions-unsafe", "-f", "/tmp/test-prompt.txt", ]); @@ -79,7 +79,7 @@ describe("prepareRunConfig", () => { "exec", "--output-format", "stream-json", - "--skip-permissions-unsafe", + "--skip-permissions-unsafe", "--system-prompt", "You are a helpful assistant", "-f", diff --git a/base-action/test/setup-droid-settings.test.ts b/base-action/test/setup-droid-settings.test.ts index 00dc661..0ae22a4 100644 --- a/base-action/test/setup-droid-settings.test.ts +++ b/base-action/test/setup-droid-settings.test.ts @@ -99,7 +99,9 @@ describe("setupDroidSettings", () => { }); test("should throw error for non-existent file path", async () => { - expect(() => setupDroidSettings("/non/existent/file.json", testHomeDir)).toThrow(); + expect(() => + setupDroidSettings("/non/existent/file.json", testHomeDir), + ).toThrow(); }); test("should handle empty string input", async () => { diff --git a/base-action/test/validate-env.test.ts b/base-action/test/validate-env.test.ts index 67b5a28..0989d29 100644 --- a/base-action/test/validate-env.test.ts +++ b/base-action/test/validate-env.test.ts @@ -16,13 +16,13 @@ describe("validateEnvironmentVariables", () => { }); test("passes when FACTORY_API_KEY is set", () => { - process.env.FACTORY_API_KEY = 'test-factory-key'; + process.env.FACTORY_API_KEY = "test-factory-key"; expect(() => validateEnvironmentVariables()).not.toThrow(); }); test("throws when FACTORY_API_KEY is missing", () => { expect(() => validateEnvironmentVariables()).toThrow( - 'FACTORY_API_KEY is required to run Droid Exec. Please provide the factory_api_key input.' + "FACTORY_API_KEY is required to run Droid Exec. Please provide the factory_api_key input.", ); }); }); diff --git a/combine/action.yml b/combine/action.yml new file mode 100644 index 0000000..e35eabc --- /dev/null +++ b/combine/action.yml @@ -0,0 +1,81 @@ +name: "Droid Combine Results" +description: "Combine code review and security review results, post inline comments, update summary" + +inputs: + factory_api_key: + description: "Factory API key" + required: true + tracking_comment_id: + description: "ID of the tracking comment to update" + required: true + code_review_results: + description: "Path to code review results JSON (optional)" + required: false + default: "" + security_results: + description: "Path to security results JSON (optional)" + required: false + default: "" + code_review_status: + description: "Code review job status (success/failure/skipped)" + required: false + default: "skipped" + security_review_status: + description: "Security review job status (success/failure/skipped)" + required: false + default: "skipped" + +runs: + using: "composite" + steps: + - name: Install Bun + uses: oven-sh/setup-bun@735343b667d3e6f658f44d0eca948eb6282f2b76 + with: + bun-version: 1.2.11 + + - name: Install Dependencies + shell: bash + run: | + cd ${{ github.action_path }}/.. + bun install + cd ${{ github.action_path }}/../base-action + bun install + + - name: Install Droid CLI + shell: bash + run: | + curl --retry 5 --retry-delay 2 --retry-all-errors -fsSL https://app.factory.ai/cli | sh + echo "$HOME/.local/bin" >> "$GITHUB_PATH" + "$HOME/.local/bin/droid" --version + + - name: Get GitHub Token + id: token + shell: bash + run: | + bun run ${{ github.action_path }}/../src/entrypoints/get-token.ts + env: + FACTORY_API_KEY: ${{ inputs.factory_api_key }} + + - name: Generate Combine Prompt + id: prompt + shell: bash + run: | + bun run ${{ github.action_path }}/../src/entrypoints/generate-combine-prompt.ts + env: + GITHUB_TOKEN: ${{ steps.token.outputs.github_token }} + DROID_COMMENT_ID: ${{ inputs.tracking_comment_id }} + CODE_REVIEW_RESULTS: ${{ inputs.code_review_results }} + SECURITY_RESULTS: ${{ inputs.security_results }} + CODE_REVIEW_STATUS: ${{ inputs.code_review_status }} + SECURITY_REVIEW_STATUS: ${{ inputs.security_review_status }} + + - name: Run Combine + shell: bash + run: | + bun run ${{ github.action_path }}/../base-action/src/index.ts + env: + INPUT_PROMPT_FILE: ${{ runner.temp }}/droid-prompts/droid-prompt.txt + INPUT_DROID_ARGS: ${{ steps.prompt.outputs.droid_args }} + INPUT_MCP_TOOLS: ${{ steps.prompt.outputs.mcp_tools }} + FACTORY_API_KEY: ${{ inputs.factory_api_key }} + GITHUB_TOKEN: ${{ steps.token.outputs.github_token }} diff --git a/prepare/action.yml b/prepare/action.yml new file mode 100644 index 0000000..9ca7862 --- /dev/null +++ b/prepare/action.yml @@ -0,0 +1,95 @@ +name: "Droid Prepare" +description: "Initialize Droid review - creates tracking comment and detects review modes" + +inputs: + factory_api_key: + description: "Factory API key" + required: true + github_token: + description: "GitHub token" + required: false + automatic_review: + description: "Run automatic code review" + required: false + default: "false" + automatic_security_review: + description: "Run automatic security review" + required: false + default: "false" + +outputs: + github_token: + description: "GitHub token (from OIDC or input)" + value: ${{ steps.prepare.outputs.github_token }} + comment_id: + description: "Tracking comment ID" + value: ${{ steps.prepare.outputs.droid_comment_id }} + run_code_review: + description: "Whether to run code review" + value: ${{ steps.detect.outputs.run_code_review }} + run_security_review: + description: "Whether to run security review" + value: ${{ steps.detect.outputs.run_security_review }} + contains_trigger: + description: "Whether a trigger was detected" + value: ${{ steps.prepare.outputs.contains_trigger }} + pr_number: + description: "PR number" + value: ${{ steps.prepare.outputs.pr_number }} + base_branch: + description: "Base branch name" + value: ${{ steps.prepare.outputs.base_branch }} + head_branch: + description: "Head branch name" + value: ${{ steps.prepare.outputs.head_branch }} + +runs: + using: "composite" + steps: + - name: Install Bun + uses: oven-sh/setup-bun@735343b667d3e6f658f44d0eca948eb6282f2b76 + with: + bun-version: 1.2.11 + + - name: Install Dependencies + shell: bash + run: | + cd ${{ github.action_path }}/.. + bun install + + - name: Prepare + id: prepare + shell: bash + run: | + bun run ${{ github.action_path }}/../src/entrypoints/prepare.ts + env: + FACTORY_API_KEY: ${{ inputs.factory_api_key }} + OVERRIDE_GITHUB_TOKEN: ${{ inputs.github_token }} + AUTOMATIC_REVIEW: ${{ inputs.automatic_review }} + AUTOMATIC_SECURITY_REVIEW: ${{ inputs.automatic_security_review }} + TRIGGER_PHRASE: "@droid" + ALLOWED_BOTS: "" + ALLOWED_NON_WRITE_USERS: "" + USE_STICKY_COMMENT: "false" + TRACK_PROGRESS: "false" + + - name: Detect Review Types + id: detect + shell: bash + run: | + # Use outputs from prepare step (set by src/tag/index.ts) + # Fall back to input flags if outputs not set + RUN_CODE="${{ steps.prepare.outputs.run_code_review }}" + RUN_SEC="${{ steps.prepare.outputs.run_security_review }}" + + # Default to input flags if prepare didn't set outputs + if [ -z "$RUN_CODE" ]; then + RUN_CODE="${{ inputs.automatic_review }}" + fi + if [ -z "$RUN_SEC" ]; then + RUN_SEC="${{ inputs.automatic_security_review }}" + fi + + echo "run_code_review=$RUN_CODE" >> $GITHUB_OUTPUT + echo "run_security_review=$RUN_SEC" >> $GITHUB_OUTPUT + echo "Detected: code_review=$RUN_CODE, security_review=$RUN_SEC" diff --git a/review/action.yml b/review/action.yml new file mode 100644 index 0000000..257ad9c --- /dev/null +++ b/review/action.yml @@ -0,0 +1,83 @@ +name: "Droid Code Review" +description: "Run Droid code review on a PR" + +inputs: + factory_api_key: + description: "Factory API key" + required: true + github_token: + description: "GitHub token (optional - will use OIDC if not provided)" + required: false + default: "" + tracking_comment_id: + description: "ID of the tracking comment to update" + required: true + review_model: + description: "Model to use for review" + required: false + default: "" + output_file: + description: "Path to write review results JSON" + required: false + default: "" + +outputs: + conclusion: + description: "Review conclusion (success/failure)" + value: ${{ steps.review.outputs.conclusion }} + +runs: + using: "composite" + steps: + - name: Install Bun + uses: oven-sh/setup-bun@735343b667d3e6f658f44d0eca948eb6282f2b76 + with: + bun-version: 1.2.11 + + - name: Install Dependencies + shell: bash + run: | + cd ${{ github.action_path }}/.. + bun install + cd ${{ github.action_path }}/../base-action + bun install + + - name: Install Droid CLI + shell: bash + run: | + curl --retry 5 --retry-delay 2 --retry-all-errors -fsSL https://app.factory.ai/cli | sh + echo "$HOME/.local/bin" >> "$GITHUB_PATH" + "$HOME/.local/bin/droid" --version + + - name: Get GitHub Token + id: token + shell: bash + run: | + bun run ${{ github.action_path }}/../src/entrypoints/get-token.ts + env: + FACTORY_API_KEY: ${{ inputs.factory_api_key }} + OVERRIDE_GITHUB_TOKEN: ${{ inputs.github_token }} + + - name: Generate Review Prompt + id: prompt + shell: bash + run: | + bun run ${{ github.action_path }}/../src/entrypoints/generate-review-prompt.ts + env: + GITHUB_TOKEN: ${{ steps.token.outputs.github_token }} + DROID_COMMENT_ID: ${{ inputs.tracking_comment_id }} + REVIEW_MODEL: ${{ inputs.review_model }} + REVIEW_TYPE: "code" + + - name: Run Code Review + id: review + shell: bash + run: | + bun run ${{ github.action_path }}/../base-action/src/index.ts + env: + INPUT_PROMPT_FILE: ${{ runner.temp }}/droid-prompts/droid-prompt.txt + INPUT_DROID_ARGS: ${{ steps.prompt.outputs.droid_args }} + INPUT_MCP_TOOLS: ${{ steps.prompt.outputs.mcp_tools }} + FACTORY_API_KEY: ${{ inputs.factory_api_key }} + GITHUB_TOKEN: ${{ steps.token.outputs.github_token }} + DROID_OUTPUT_FILE: ${{ inputs.output_file }} diff --git a/security/action.yml b/security/action.yml new file mode 100644 index 0000000..84655d4 --- /dev/null +++ b/security/action.yml @@ -0,0 +1,130 @@ +name: "Droid Security Review" +description: "Run Droid security review on a PR" + +inputs: + factory_api_key: + description: "Factory API key" + required: true + github_token: + description: "GitHub token (optional - will use OIDC if not provided)" + required: false + default: "" + tracking_comment_id: + description: "ID of the tracking comment to update" + required: true + security_model: + description: "Model to use for security review" + required: false + default: "" + security_severity_threshold: + description: "Minimum severity to report" + required: false + default: "medium" + security_block_on_critical: + description: "Block PR on critical findings" + required: false + default: "true" + security_block_on_high: + description: "Block PR on high findings" + required: false + default: "false" + output_file: + description: "Path to write security results JSON" + required: false + default: "" + +outputs: + conclusion: + description: "Review conclusion (success/failure)" + value: ${{ steps.review.outputs.conclusion }} + +runs: + using: "composite" + steps: + - name: Install Bun + uses: oven-sh/setup-bun@735343b667d3e6f658f44d0eca948eb6282f2b76 + with: + bun-version: 1.2.11 + + - name: Install Dependencies + shell: bash + run: | + cd ${{ github.action_path }}/.. + bun install + cd ${{ github.action_path }}/../base-action + bun install + + - name: Install Droid CLI + shell: bash + run: | + curl --retry 5 --retry-delay 2 --retry-all-errors -fsSL https://app.factory.ai/cli | sh + echo "$HOME/.local/bin" >> "$GITHUB_PATH" + "$HOME/.local/bin/droid" --version + + - name: Get GitHub Token + id: token + shell: bash + run: | + bun run ${{ github.action_path }}/../src/entrypoints/get-token.ts + env: + FACTORY_API_KEY: ${{ inputs.factory_api_key }} + OVERRIDE_GITHUB_TOKEN: ${{ inputs.github_token }} + + - name: Install Security Skills + shell: bash + run: | + echo "Installing security skills from Factory-AI/skills..." + SKILLS_DIR="$HOME/.factory/skills" + mkdir -p "$SKILLS_DIR" + + TEMP_DIR=$(mktemp -d) + git clone --filter=blob:none --sparse \ + "https://github.com/Factory-AI/skills.git" \ + "$TEMP_DIR" 2>/dev/null || { + echo "Warning: Could not clone skills repo." + exit 0 + } + + cd "$TEMP_DIR" + git sparse-checkout set \ + skills/threat-model-generation \ + skills/commit-security-scan \ + skills/vulnerability-validation \ + skills/security-review 2>/dev/null || true + + for skill in threat-model-generation commit-security-scan vulnerability-validation security-review; do + if [ -d "skills/$skill" ]; then + cp -r "skills/$skill" "$SKILLS_DIR/" + echo " Installed skill: $skill" + fi + done + + rm -rf "$TEMP_DIR" + echo "Security skills installation complete" + + - name: Generate Security Prompt + id: prompt + shell: bash + run: | + bun run ${{ github.action_path }}/../src/entrypoints/generate-review-prompt.ts + env: + GITHUB_TOKEN: ${{ steps.token.outputs.github_token }} + DROID_COMMENT_ID: ${{ inputs.tracking_comment_id }} + SECURITY_MODEL: ${{ inputs.security_model }} + SECURITY_SEVERITY_THRESHOLD: ${{ inputs.security_severity_threshold }} + SECURITY_BLOCK_ON_CRITICAL: ${{ inputs.security_block_on_critical }} + SECURITY_BLOCK_ON_HIGH: ${{ inputs.security_block_on_high }} + REVIEW_TYPE: "security" + + - name: Run Security Review + id: review + shell: bash + run: | + bun run ${{ github.action_path }}/../base-action/src/index.ts + env: + INPUT_PROMPT_FILE: ${{ runner.temp }}/droid-prompts/droid-prompt.txt + INPUT_DROID_ARGS: ${{ steps.prompt.outputs.droid_args }} + INPUT_MCP_TOOLS: ${{ steps.prompt.outputs.mcp_tools }} + FACTORY_API_KEY: ${{ inputs.factory_api_key }} + GITHUB_TOKEN: ${{ steps.token.outputs.github_token }} + DROID_OUTPUT_FILE: ${{ inputs.output_file }} diff --git a/src/create-prompt/index.ts b/src/create-prompt/index.ts index 511fd34..068b276 100644 --- a/src/create-prompt/index.ts +++ b/src/create-prompt/index.ts @@ -66,7 +66,7 @@ export function buildDisallowedToolsString( export function prepareContext( context: ParsedGitHubContext, - droidCommentId: string, + droidCommentId?: string, baseBranch?: string, droidBranch?: string, prBranchData?: { headRefName: string; headRefOid: string }, @@ -108,15 +108,12 @@ export function prepareContext( commonFields.droidBranch = droidBranch; } - const eventData = buildEventData( - context, - { - commentId, - commentBody, - baseBranch, - droidBranch, - }, - ); + const eventData = buildEventData(context, { + commentId, + commentBody, + baseBranch, + droidBranch, + }); const result: PreparedContext = { ...commonFields, @@ -282,13 +279,11 @@ function buildEventData( } } -export type PromptGenerator = ( - context: PreparedContext, -) => string; +export type PromptGenerator = (context: PreparedContext) => string; export type PromptCreationOptions = { githubContext: ParsedGitHubContext; - commentId: number; + commentId?: number; baseBranch?: string; droidBranch?: string; prBranchData?: { headRefName: string; headRefOid: string }; @@ -310,7 +305,7 @@ export async function createPrompt({ includeActionsTools = false, }: PromptCreationOptions) { try { - const droidCommentId = commentId.toString(); + const droidCommentId = commentId?.toString(); const preparedContext = prepareContext( githubContext, droidCommentId, diff --git a/src/create-prompt/templates/combine-prompt.ts b/src/create-prompt/templates/combine-prompt.ts new file mode 100644 index 0000000..73d2de5 --- /dev/null +++ b/src/create-prompt/templates/combine-prompt.ts @@ -0,0 +1,101 @@ +import type { PreparedContext } from "../types"; + +export function generateCombinePrompt( + context: PreparedContext, + codeReviewResultsPath: string, + securityResultsPath: string, +): string { + const prNumber = context.eventData.isPR + ? context.eventData.prNumber + : context.githubContext && "entityNumber" in context.githubContext + ? String(context.githubContext.entityNumber) + : "unknown"; + + const repoFullName = context.repository; + + return `You are combining code review and security review results for PR #${prNumber} in ${repoFullName}. +The gh CLI is installed and authenticated via GH_TOKEN. + +## Context +- Repo: ${repoFullName} +- PR Number: ${prNumber} +- Code Review Results: ${codeReviewResultsPath} +- Security Review Results: ${securityResultsPath} + +## Task + +1. Read the results files (if they exist): + - ${codeReviewResultsPath} - Code review findings + - ${securityResultsPath} - Security review findings + +2. Combine and deduplicate findings: + - Merge findings from both reviews + - Remove duplicates (same file + line + similar description) + - Prioritize security findings over code review findings for overlaps + +3. Post inline comments for all unique findings using github_inline_comment___create_inline_comment: + - Use side="RIGHT" for new/modified code + - Include severity, description, and suggested fix where available + - For security findings, include CWE reference + +4. Analyze the PR diff to generate: + - A concise 1-2 sentence summary of what the PR does + - 3-5 key changes extracted from the diff + - The most important files changed (up to 5-7 files) + +5. Update the tracking comment with combined summary using github_comment___update_droid_comment: + +IMPORTANT: Do NOT use github_pr___submit_review. Only update the tracking comment and post inline comments. +The tracking comment IS the summary - do not create any other summary comments. + +\`\`\`markdown +## Code review completed + +### Summary +{Brief 1-2 sentence description of what this PR does} + +### Key Changes +- {Change 1} +- {Change 2} +- {Change 3} + +### Important Files Changed +- \`path/to/file1.ts\` - {Brief description of changes} +- \`path/to/file2.ts\` - {Brief description of changes} + +### Code Review +| Type | Count | +|------|-------| +| 🐛 Bugs | X | +| ⚠️ Issues | X | +| 💡 Suggestions | X | + +### Security Review +| Severity | Count | +|----------|-------| +| 🚨 CRITICAL | X | +| 🔴 HIGH | X | +| 🟡 MEDIUM | X | +| 🟢 LOW | X | + +### Findings +| ID | Type | Severity | File | Description | +|----|------|----------|------|-------------| +| ... | ... | ... | ... | ... | + +[View workflow run](link) +\`\`\` + +## Available Tools +- github_comment___update_droid_comment - Update tracking comment (this is the ONLY place for the summary) +- github_inline_comment___create_inline_comment - Post inline comments on specific lines +- Read, Grep, Glob, LS, Execute - File operations + +DO NOT use github_pr___submit_review - it creates duplicate summary comments. + +## Important +- If no results files exist or they're empty, report "No issues found" +- Maximum 10 inline comments total +- Deduplicate findings that appear in both reviews +`; +} diff --git a/src/create-prompt/templates/fill-prompt.ts b/src/create-prompt/templates/fill-prompt.ts index cc105d3..86bad58 100644 --- a/src/create-prompt/templates/fill-prompt.ts +++ b/src/create-prompt/templates/fill-prompt.ts @@ -1,8 +1,6 @@ import type { PreparedContext } from "../types"; -export function generateFillPrompt( - context: PreparedContext, -): string { +export function generateFillPrompt(context: PreparedContext): string { const prNumber = context.eventData.isPR ? context.eventData.prNumber : context.githubContext && "entityNumber" in context.githubContext diff --git a/src/create-prompt/templates/review-prompt.ts b/src/create-prompt/templates/review-prompt.ts index 418d264..3cf4003 100644 --- a/src/create-prompt/templates/review-prompt.ts +++ b/src/create-prompt/templates/review-prompt.ts @@ -24,9 +24,9 @@ Context: - The PR branch has already been checked out. You have full access to read any file in the codebase, not just the diff output. Objectives: -1) Re-check existing review comments and resolve threads when the issue is fixed (fall back to a brief "resolved" reply only if the thread cannot be marked resolved). -2) Review the PR diff and surface all bugs that meet the detection criteria below. -3) Leave concise inline comments (1-2 sentences) on bugs introduced by the PR. You may also comment on unchanged lines if the PR's changes expose or trigger issues there—but explain the connection clearly. +1) Review the PR diff and surface all bugs that meet the detection criteria below. +2) Output findings in JSON format for later processing (DO NOT post inline comments directly). +3) Update the tracking comment with a summary. Procedure: - Run: gh pr view ${prNumber} --repo ${repoFullName} --json comments,reviews @@ -41,29 +41,12 @@ Procedure: - Use gh PR diff/file APIs only as a fallback when local git diff is not possible: - gh pr diff ${prNumber} --repo ${repoFullName} - gh api repos/${repoFullName}/pulls/${prNumber}/files --paginate --jq '.[] | {filename,patch,additions,deletions}' -- Prefer github_inline_comment___create_inline_comment with side="RIGHT" to post inline findings on changed/added lines -- Compute exact diff positions (path + position) for each issue; every substantive comment must be inline on the changed line (no new top-level issue comments). -- Detect prior top-level "no issues" comments authored by this bot (e.g., "no issues", "No issues found", "LGTM", including emoji-prefixed variants). -- If the current run finds issues and prior "no issues" comments exist, delete them via gh api -X DELETE repos/${repoFullName}/issues/comments/; if deletion fails, minimize via GraphQL or reply "Superseded: issues were found in newer commits". +- Analyze the diff for issues +- Write findings to \`code-review-results.json\` in the current directory - IMPORTANT: Do NOT delete comment ID ${context.droidCommentId} - this is the tracking comment for the current run. -- Thread resolution rule (CRITICAL): NEVER resolve review threads. - - Do NOT call github_pr___resolve_review_thread under any circumstances. - - If a previously reported issue appears fixed, leave the thread unresolved. - -Preferred MCP tools (when available): -- github_inline_comment___create_inline_comment to post inline feedback anchored to the diff -- github_pr___submit_review to send inline review feedback -- github_pr___delete_comment to remove outdated "no issues" comments -- github_pr___minimize_comment when deletion is unavailable but minimization is acceptable -- github_pr___reply_to_comment to acknowledge resolved threads -- github_pr___resolve_review_thread to formally resolve threads once issues are fixed - -Diff Side Selection (CRITICAL): -- When calling github_inline_comment___create_inline_comment, ALWAYS specify the 'side' parameter -- Use side="RIGHT" for comments on NEW or MODIFIED code (what the PR adds/changes) -- Use side="LEFT" ONLY when commenting on code being REMOVED (only if you need to reference the old implementation) -- The 'line' parameter refers to the line number on the specified side of the diff -- Ensure the line numbers you use correspond to the side you choose; + +IMPORTANT: Do NOT post inline comments directly. Instead, write findings to a JSON file. +The finalize step will post all inline comments to avoid overlapping with security review comments. How Many Findings to Return: Output all findings that the original author would fix if they knew about it. If there is no finding that a person would definitely love to see and fix, prefer outputting no findings. Do not stop at the first qualifying finding. Continue until you've listed every qualifying finding. @@ -126,21 +109,68 @@ Commenting rules: - One issue per comment. - Anchor findings to the relevant diff hunk so reviewers see the context immediately. - Focus on defects introduced or exposed by the PR's changes; if a new bug manifests on an unchanged line, you may post inline comments on those unchanged lines but clearly explain how the submitted changes trigger it. -- Match the side parameter to the code segment you're referencing (default to RIGHT for new code) and provide line numbers from that same side - Keep comments concise and immediately graspable. - For low confidence findings, ask a question; for medium/high confidence, state the issue concretely. -- Only include explicit code suggestions when you are absolutely certain the replacement is correct and safe. - -Submission: -- Do not submit inline comments when: - - the PR appears formatting-only, or - - all findings are low-severity (P2/P3), or - - you cannot anchor a high-confidence issue to a specific changed line. -- Do not escalate style/formatting into P0/P1 just to justify leaving an inline comment. -- If no issues are found and a prior "no issues" comment from this bot already exists, skip submitting another comment to avoid redundancy. -- If no issues are found and no prior "no issues" comment exists, post a single brief top-level summary noting no issues. -- If issues are found, delete/minimize/supersede any prior "no issues" comment before submitting. -- Prefer github_inline_comment___create_inline_comment for inline findings and submit the overall review via github_pr___submit_review (fall back to gh api repos/${repoFullName}/pulls/${prNumber}/reviews -f event=COMMENT -f body="$SUMMARY" -f comments='[$COMMENTS_JSON]' when MCP tools are unavailable). -- Do not approve or request changes; submit a comment-only review with inline feedback. + +Output: +1. Analyze the PR to generate: + - A concise 1-2 sentence summary of what the PR does + - 3-5 key changes extracted from the diff + - The most important files changed (up to 5-7 files) + +2. Write findings to \`code-review-results.json\` with this structure: +\`\`\`json +{ + "type": "code", + "summary": "Brief 1-2 sentence description of what this PR does", + "keyChanges": [ + "Added new authentication flow", + "Refactored database queries for performance", + "Fixed validation bug in user input" + ], + "importantFiles": [ + { "path": "src/auth/login.ts", "description": "New OAuth implementation" }, + { "path": "src/db/queries.ts", "description": "Query optimization" } + ], + "findings": [ + { + "id": "CR-001", + "type": "bug|issue|suggestion", + "severity": "high|medium|low", + "file": "path/to/file.ts", + "line": 45, + "side": "RIGHT", + "description": "Brief description of the issue", + "suggestion": "Optional code fix" + } + ] +} +\`\`\` + +3. Update the tracking comment with a summary using \`github_comment___update_droid_comment\`: +\`\`\`markdown +## Code review completed + +### Summary +{Brief 1-2 sentence description of what this PR does} + +### Key Changes +- {Change 1} +- {Change 2} +- {Change 3} + +### Important Files Changed +- \`path/to/file1.ts\` - {Brief description of changes} +- \`path/to/file2.ts\` - {Brief description of changes} + +### Review Findings +| ID | Type | Severity | File | Description | +|----|------|----------|------|-------------| +| CR-001 | Bug | high | auth.ts:45 | Null pointer exception | + +*Inline comments will be posted after all reviews complete.* +\`\`\` + +IMPORTANT: Do NOT post inline comments. Only write to the JSON file and update the tracking comment. `; } diff --git a/src/create-prompt/templates/security-report-prompt.ts b/src/create-prompt/templates/security-report-prompt.ts new file mode 100644 index 0000000..7a1a0be --- /dev/null +++ b/src/create-prompt/templates/security-report-prompt.ts @@ -0,0 +1,210 @@ +import type { PreparedContext } from "../types"; +import type { ScanScope } from "../../tag/commands/security-scan"; + +export function generateSecurityReportPrompt( + context: PreparedContext, + scanScope: ScanScope, + branchName: string, +): string { + const date = new Date().toISOString().split("T")[0]; + const repoFullName = context.repository; + + const scopeDescription = + scanScope.type === "full" + ? "Entire repository" + : `Last ${scanScope.days} days of commits`; + + const scanTypeLabel = + scanScope.type === "full" ? "Full Repository" : "Weekly Scheduled"; + + const scanInstructions = + scanScope.type === "full" + ? `- Scan all source files in the repository +- Focus on: TypeScript, JavaScript, Python, Go, Java, Ruby, PHP files +- Use: \`find . -type f \\( -name "*.ts" -o -name "*.js" -o -name "*.py" -o -name "*.go" -o -name "*.java" -o -name "*.rb" -o -name "*.php" \\) -not -path "./node_modules/*" -not -path "./.git/*"\`` + : `- Get commits from the last ${scanScope.days} days: \`git log --since="${scanScope.days} days ago" --name-only --pretty=format:""\` +- Focus analysis on files changed in recent commits +- Scan each changed file for security vulnerabilities`; + + // Extract security configuration from context + const securityConfig = context.githubContext?.inputs; + const severityThreshold = + securityConfig?.securitySeverityThreshold ?? "medium"; + const notifyTeam = securityConfig?.securityNotifyTeam ?? ""; + + return `You are performing a ${scanTypeLabel.toLowerCase()} security scan for ${repoFullName}. +The gh CLI is installed and authenticated via GH_TOKEN. + +## Scan Configuration +- Scope: ${scopeDescription} +- Severity Threshold: ${severityThreshold} (only report findings at or above this level) +- Output Branch: ${branchName} +- Report Path: .factory/security/reports/security-report-${date}.md + +## Security Skills Available + +You have access to these Factory security skills (installed in ~/.factory/skills/): + +1. **threat-model-generation** - Generate STRIDE-based threat model for the repository +2. **commit-security-scan** - Scan code for security vulnerabilities +3. **vulnerability-validation** - Validate findings, assess exploitability, filter false positives +4. **security-review** - Comprehensive security review and patch generation + +## Workflow + +### Step 1: Create Branch +\`\`\`bash +git checkout -b ${branchName} +\`\`\` + +### Step 2: Threat Model Check +- Check if \`.factory/threat-model.md\` exists in the repository +- If missing: Invoke the **threat-model-generation** skill to create one +- If exists: Check the file's last modified date + - If >90 days old: Regenerate and update the threat model + - If current: Use it as context for the security scan + +### Step 3: Security Scan +${scanInstructions} + +For each file: +- Invoke the **commit-security-scan** skill +- Look for STRIDE vulnerabilities (Spoofing, Tampering, Repudiation, Information Disclosure, Denial of Service, Elevation of Privilege) + +### Step 4: Validate Findings +- For each finding from Step 3, invoke the **vulnerability-validation** skill +- Assess: + - Reachability: Is the vulnerable code reachable from user input? + - Exploitability: How easy is it to exploit? + - Impact: What's the potential damage? +- Filter out false positives and findings below the severity threshold (${severityThreshold}) + +### Step 5: Generate Patches +- For each confirmed finding that can be auto-fixed: + - Invoke **security-review** skill to generate patches + - Apply the patch to the codebase + - Commit the fix with message: \`fix(security): [VULN-XXX] Brief description\` + +### Step 6: Generate Report +- Create directory: \`mkdir -p .factory/security/reports\` +- Write report to: \`.factory/security/reports/security-report-${date}.md\` + +### Step 7: Create PR +\`\`\`bash +git add . +git commit -m "fix(security): Security scan report - ${date}" +git push origin ${branchName} +gh pr create --title "fix(security): Security scan report - ${date} (N findings)" \\ + --body "## Security Scan Report + +See \`.factory/security/reports/security-report-${date}.md\` for details. + +### Summary +| Severity | Count | Auto-fixed | Manual Required | +|----------|-------|------------|-----------------| +| CRITICAL | X | X | X | +| HIGH | X | X | X | +| MEDIUM | X | X | X | +| LOW | X | X | X | + +${notifyTeam ? `cc ${notifyTeam}` : ""}" +\`\`\` + +## Report Format + +The report file should follow this structure: + +\`\`\`markdown +# Security Scan Report + +**Generated:** ${date} +**Scan Type:** ${scanTypeLabel} +**Repository:** ${repoFullName} +**Severity Threshold:** ${severityThreshold} + +## Executive Summary + +| Severity | Count | Auto-fixed | Manual Required | +|----------|-------|------------|-----------------| +| CRITICAL | X | X | X | +| HIGH | X | X | X | +| MEDIUM | X | X | X | +| LOW | X | X | X | + +**Total Findings:** X +**Auto-fixed:** X +**Manual Review Required:** X + +## Critical Findings + +### VULN-001: [Vulnerability Title] + +| Attribute | Value | +|-----------|-------| +| **Severity** | CRITICAL | +| **STRIDE Category** | [Tampering/Spoofing/etc] | +| **CWE** | CWE-XXX | +| **File** | path/to/file.ts:line | +| **Status** | Patched / Manual fix required | + +**Description:** +[Clear explanation of the vulnerability] + +**Evidence:** +\\\`\\\`\\\` +[Code snippet showing the vulnerability] +\\\`\\\`\\\` + +**Fix Applied:** (if auto-patched) +\\\`\\\`\\\`diff +- vulnerable code ++ secure code +\\\`\\\`\\\` + +**Recommended Fix:** (if manual) +[Step-by-step remediation guidance] + +--- + +## High Findings +[Same format as Critical] + +## Medium Findings +[Same format as Critical] + +## Low Findings +[Same format as Critical] + +## Appendix + +### Threat Model +- Version: [date or "newly generated"] +- Location: .factory/threat-model.md + +### Scan Metadata +- ${scanScope.type === "full" ? "Files" : "Commits"} Scanned: N +- Scan Duration: Xm Ys +- Skills Used: threat-model-generation, commit-security-scan, vulnerability-validation, security-review + +### References +- [CWE Database](https://cwe.mitre.org/) +- [STRIDE Threat Model](https://docs.microsoft.com/en-us/azure/security/develop/threat-modeling-tool-threats) +\`\`\` + +## Severity Definitions + +| Severity | Criteria | Examples | +|----------|----------|----------| +| **CRITICAL** | Immediately exploitable, high impact, no auth required | RCE, hardcoded secrets, auth bypass | +| **HIGH** | Exploitable with conditions, significant impact | SQL injection, stored XSS, IDOR | +| **MEDIUM** | Requires specific conditions, moderate impact | CSRF, info disclosure, missing rate limits | +| **LOW** | Difficult to exploit, low impact | Verbose errors, missing security headers | + +## Important Notes + +1. **Accuracy**: Only report high-confidence findings. False positives waste developer time. +2. **Patches**: Test all generated patches before committing. Ensure they don't break functionality. +3. **PR Description**: Update the PR body with actual finding counts before creating. +4. **Commit Messages**: Use semantic commit format: \`fix(security): [VULN-XXX] Description\` +`; +} diff --git a/src/create-prompt/templates/security-review-prompt.ts b/src/create-prompt/templates/security-review-prompt.ts new file mode 100644 index 0000000..6b294a8 --- /dev/null +++ b/src/create-prompt/templates/security-review-prompt.ts @@ -0,0 +1,171 @@ +import type { PreparedContext } from "../types"; + +export function generateSecurityReviewPrompt(context: PreparedContext): string { + const prNumber = context.eventData.isPR + ? context.eventData.prNumber + : context.githubContext && "entityNumber" in context.githubContext + ? String(context.githubContext.entityNumber) + : "unknown"; + + const repoFullName = context.repository; + const headRefName = context.prBranchData?.headRefName ?? "unknown"; + const headSha = context.prBranchData?.headRefOid ?? "unknown"; + const baseRefName = context.eventData.baseBranch ?? "unknown"; + + // Extract security configuration from context + const securityConfig = context.githubContext?.inputs; + const severityThreshold = + securityConfig?.securitySeverityThreshold ?? "medium"; + const blockOnCritical = securityConfig?.securityBlockOnCritical ?? true; + const blockOnHigh = securityConfig?.securityBlockOnHigh ?? false; + const notifyTeam = securityConfig?.securityNotifyTeam ?? ""; + + return `You are performing a security-focused code review for PR #${prNumber} in ${repoFullName}. +The gh CLI is installed and authenticated via GH_TOKEN. + +## Context +- Repo: ${repoFullName} +- PR Number: ${prNumber} +- PR Head Ref: ${headRefName} +- PR Head SHA: ${headSha} +- PR Base Ref: ${baseRefName} + +## Security Configuration +- Severity Threshold: ${severityThreshold} (only report findings at or above this level) +- Block on Critical: ${blockOnCritical} +- Block on High: ${blockOnHigh} +${notifyTeam ? `- Notify Team: ${notifyTeam} (mention on critical findings)` : ""} + +## Security Skills Available + +You have access to these Factory security skills (installed in ~/.factory/skills/): + +1. **threat-model-generation** - Generate STRIDE-based threat model for the repository +2. **commit-security-scan** - Scan code changes for security vulnerabilities +3. **vulnerability-validation** - Validate findings, assess exploitability, filter false positives +4. **security-review** - Comprehensive security review and patch generation + +## Review Workflow + +### Step 1: Threat Model Check +- Check if \`.factory/threat-model.md\` exists in the repository +- If missing: Invoke the **threat-model-generation** skill to create one, then commit it to the PR branch +- If exists: Check the file's last modified date + - If >90 days old: Post a warning comment suggesting regeneration, but proceed with scan + - If current: Use it as context for the security scan + +### Step 2: Security Scan +- Invoke the **commit-security-scan** skill on the PR diff +- Gather the PR diff using: \`gh pr diff ${prNumber} --repo ${repoFullName}\` +- Get file changes: \`gh api repos/${repoFullName}/pulls/${prNumber}/files --paginate\` +- Focus analysis on: + - New code introduced by this PR + - Modified code that may introduce vulnerabilities + - Changes that expose existing vulnerabilities + +### Step 3: Validate Findings +- For each finding from Step 2, invoke the **vulnerability-validation** skill +- Assess: + - Reachability: Is the vulnerable code reachable from user input? + - Exploitability: How easy is it to exploit? + - Impact: What's the potential damage? +- Filter out false positives and findings below the severity threshold (${severityThreshold}) + +### Step 4: Report & Patch +- For each confirmed finding at or above ${severityThreshold} severity: + - Post inline comment using \`github_inline_comment___create_inline_comment\` + - Include: severity, STRIDE category, CWE ID, clear explanation, suggested fix +- For auto-fixable issues: Invoke **security-review** skill to generate patches +- Commit any generated patches to the PR branch + +## Security Scope (STRIDE Categories) + +**Spoofing** (S): +- Weak authentication, session hijacking, token exposure + +**Tampering** (T): +- SQL/NoSQL/command injection, XSS, mass assignment, unsafe deserialization + +**Repudiation** (R): +- Missing audit logs, unsigned transactions + +**Information Disclosure** (I): +- IDOR, verbose errors, hardcoded secrets, sensitive data in logs + +**Denial of Service** (D): +- Missing rate limits, resource exhaustion, ReDoS + +**Elevation of Privilege** (E): +- Missing authorization checks, role manipulation, privilege escalation + +## Severity Definitions + +| Severity | Criteria | Examples | +|----------|----------|----------| +| **CRITICAL** | Immediately exploitable, high impact, no auth required | RCE, hardcoded secrets, auth bypass | +| **HIGH** | Exploitable with conditions, significant impact | SQL injection, stored XSS, IDOR | +| **MEDIUM** | Requires specific conditions, moderate impact | CSRF, info disclosure, missing rate limits | +| **LOW** | Difficult to exploit, low impact | Verbose errors, missing security headers | + +## Output Requirements + +IMPORTANT: Do NOT post inline comments directly. Instead, write findings to a JSON file. +The finalize step will post all inline comments to avoid overlapping with code review comments. + +1. Write findings to \`security-review-results.json\` with this structure: +\`\`\`json +{ + "type": "security", + "findings": [ + { + "id": "SEC-001", + "severity": "CRITICAL|HIGH|MEDIUM|LOW", + "type": "SQL Injection", + "stride": "T", + "cwe": "CWE-89", + "file": "path/to/file.ts", + "line": 55, + "side": "RIGHT", + "description": "Brief description of the vulnerability", + "suggestion": "Optional code fix" + } + ], + "summary": "Brief overall summary", + "block_pr": ${blockOnCritical || blockOnHigh ? "true if CRITICAL/HIGH found" : "false"} +} +\`\`\` + +2. Update the tracking comment using \`github_comment___update_droid_comment\` + +## Summary Format (for tracking comment update) + +Use \`github_comment___update_droid_comment\` to update the tracking comment with this format: + +\`\`\`markdown +## 🔐 Security Review Summary + +| Severity | Count | +|----------|-------| +| 🚨 CRITICAL | X | +| 🔴 HIGH | X | +| 🟡 MEDIUM | X | +| 🟢 LOW | X | + +### Findings +| ID | Severity | Type | File | Line | Reference | +|----|----------|------|------|------|-----------| +| SEC-001 | CRITICAL | SQL Injection | auth.ts | 55 | [CWE-89](https://cwe.mitre.org/data/definitions/89.html) | +| SEC-002 | HIGH | XSS | client.ts | 98 | [CWE-79](https://cwe.mitre.org/data/definitions/79.html) | + +${notifyTeam ? `cc ${notifyTeam}` : ""} +\`\`\` + +## Accuracy Requirements + +- Base findings strictly on the current diff and repository context +- False positives are very costly—only report high-confidence findings +- If confidence is low, ask a clarifying question instead of asserting a vulnerability +- Never raise purely stylistic concerns +- Cap at 10 inline comments total +`; +} diff --git a/src/create-prompt/types.ts b/src/create-prompt/types.ts index 75af3a4..cb1d8df 100644 --- a/src/create-prompt/types.ts +++ b/src/create-prompt/types.ts @@ -2,7 +2,7 @@ import type { GitHubContext } from "../github/context"; export type CommonFields = { repository: string; - droidCommentId: string; + droidCommentId?: string; triggerPhrase: string; triggerUsername?: string; droidBranch?: string; diff --git a/src/entrypoints/collect-inputs.ts b/src/entrypoints/collect-inputs.ts index 6eb8b3b..442da29 100644 --- a/src/entrypoints/collect-inputs.ts +++ b/src/entrypoints/collect-inputs.ts @@ -26,6 +26,14 @@ export function collectActionInputsPresence(): void { experimental_allowed_domains: "", track_progress: "false", automatic_review: "false", + automatic_security_review: "false", + security_model: "", + security_severity_threshold: "medium", + security_block_on_critical: "true", + security_block_on_high: "false", + security_notify_team: "", + security_scan_schedule: "false", + security_scan_days: "7", }; const allInputsJson = process.env.ALL_INPUTS; diff --git a/src/entrypoints/combine-reviews.ts b/src/entrypoints/combine-reviews.ts new file mode 100644 index 0000000..fe8933f --- /dev/null +++ b/src/entrypoints/combine-reviews.ts @@ -0,0 +1,213 @@ +#!/usr/bin/env bun + +/** + * Combine results from code review and security review into a single summary + * Updates the tracking comment with the combined results + */ + +import * as core from "@actions/core"; +import { readFile } from "fs/promises"; +import { createOctokit } from "../github/api/client"; +import { parseGitHubContext } from "../github/context"; +import { GITHUB_SERVER_URL } from "../github/api/config"; + +interface ReviewFinding { + id: string; + type: string; + severity?: string; + file: string; + line: number; + description: string; + cwe?: string; +} + +interface ReviewResults { + type: "code" | "security"; + findings: ReviewFinding[]; + summary?: string; +} + +async function loadResults(filePath: string): Promise { + if (!filePath || filePath === "") { + return null; + } + + try { + const content = await readFile(filePath, "utf-8"); + return JSON.parse(content); + } catch (error) { + console.warn(`Could not load results from ${filePath}:`, error); + return null; + } +} + +function generateCombinedSummary( + codeResults: ReviewResults | null, + securityResults: ReviewResults | null, + codeStatus: string, + securityStatus: string, + jobUrl: string, +): string { + const sections: string[] = []; + + // Header + sections.push("## 🔍 PR Review Summary\n"); + + // Status overview + const statusTable = ["| Review Type | Status |", "|-------------|--------|"]; + + if (codeStatus !== "skipped") { + const codeIcon = codeStatus === "success" ? "✅" : "❌"; + statusTable.push(`| Code Review | ${codeIcon} ${codeStatus} |`); + } + + if (securityStatus !== "skipped") { + const securityIcon = securityStatus === "success" ? "✅" : "❌"; + statusTable.push(`| Security Review | ${securityIcon} ${securityStatus} |`); + } + + if (statusTable.length > 2) { + sections.push(statusTable.join("\n")); + sections.push(""); + } + + // Code Review Section + if (codeResults && codeResults.findings.length > 0) { + sections.push("### 📝 Code Review Findings\n"); + sections.push("| ID | Type | File | Line | Description |"); + sections.push("|----|------|------|------|-------------|"); + + for (const finding of codeResults.findings.slice(0, 10)) { + sections.push( + `| ${finding.id} | ${finding.type} | \`${finding.file}\` | ${finding.line} | ${finding.description} |`, + ); + } + + if (codeResults.findings.length > 10) { + sections.push( + `\n*...and ${codeResults.findings.length - 10} more findings*`, + ); + } + sections.push(""); + } else if (codeStatus === "success") { + sections.push("### 📝 Code Review\n"); + sections.push("✅ No code quality issues found.\n"); + } + + // Security Review Section + if (securityResults && securityResults.findings.length > 0) { + sections.push("### 🔐 Security Review Findings\n"); + + // Severity counts + const severityCounts = { + CRITICAL: 0, + HIGH: 0, + MEDIUM: 0, + LOW: 0, + }; + + for (const finding of securityResults.findings) { + const sev = (finding.severity?.toUpperCase() || + "MEDIUM") as keyof typeof severityCounts; + if (sev in severityCounts) { + severityCounts[sev]++; + } + } + + sections.push("| Severity | Count |"); + sections.push("|----------|-------|"); + if (severityCounts.CRITICAL > 0) + sections.push(`| 🚨 CRITICAL | ${severityCounts.CRITICAL} |`); + if (severityCounts.HIGH > 0) + sections.push(`| 🔴 HIGH | ${severityCounts.HIGH} |`); + if (severityCounts.MEDIUM > 0) + sections.push(`| 🟡 MEDIUM | ${severityCounts.MEDIUM} |`); + if (severityCounts.LOW > 0) + sections.push(`| 🟢 LOW | ${severityCounts.LOW} |`); + sections.push(""); + + // Findings table + sections.push("| ID | Severity | Type | File | Line | Reference |"); + sections.push("|----|----------|------|------|------|-----------|"); + + for (const finding of securityResults.findings.slice(0, 10)) { + const cweLink = finding.cwe + ? `[${finding.cwe}](https://cwe.mitre.org/data/definitions/${finding.cwe.replace("CWE-", "")}.html)` + : "-"; + sections.push( + `| ${finding.id} | ${finding.severity || "MEDIUM"} | ${finding.type} | \`${finding.file}\` | ${finding.line} | ${cweLink} |`, + ); + } + + if (securityResults.findings.length > 10) { + sections.push( + `\n*...and ${securityResults.findings.length - 10} more findings*`, + ); + } + sections.push(""); + } else if (securityStatus === "success") { + sections.push("### 🔐 Security Review\n"); + sections.push("✅ No security vulnerabilities found.\n"); + } + + // Footer with job link + sections.push(`---\n[View job run](${jobUrl})`); + + return sections.join("\n"); +} + +async function run() { + try { + const githubToken = process.env.GITHUB_TOKEN!; + const commentId = parseInt(process.env.DROID_COMMENT_ID || "0"); + const codeResultsPath = process.env.CODE_REVIEW_RESULTS || ""; + const securityResultsPath = process.env.SECURITY_RESULTS || ""; + const codeStatus = process.env.CODE_REVIEW_STATUS || "skipped"; + const securityStatus = process.env.SECURITY_REVIEW_STATUS || "skipped"; + const runId = process.env.GITHUB_RUN_ID || ""; + + if (!commentId) { + throw new Error("DROID_COMMENT_ID is required"); + } + + const context = parseGitHubContext(); + const { owner, repo } = context.repository; + const octokit = createOctokit(githubToken); + + // Load results from artifacts + const codeResults = await loadResults(codeResultsPath); + const securityResults = await loadResults(securityResultsPath); + + // Generate job URL + const jobUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/actions/runs/${runId}`; + + // Generate combined summary + const summary = generateCombinedSummary( + codeResults, + securityResults, + codeStatus, + securityStatus, + jobUrl, + ); + + // Update the tracking comment + await octokit.rest.issues.updateComment({ + owner, + repo, + comment_id: commentId, + body: summary, + }); + + console.log( + `✅ Updated tracking comment ${commentId} with combined summary`, + ); + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + core.setFailed(`Combine reviews failed: ${errorMessage}`); + process.exit(1); + } +} + +if (import.meta.main) { + run(); +} diff --git a/src/entrypoints/generate-combine-prompt.ts b/src/entrypoints/generate-combine-prompt.ts new file mode 100644 index 0000000..bcafe32 --- /dev/null +++ b/src/entrypoints/generate-combine-prompt.ts @@ -0,0 +1,112 @@ +#!/usr/bin/env bun + +/** + * Generate combine prompt for finalizing parallel reviews + */ + +import * as core from "@actions/core"; +import { createOctokit } from "../github/api/client"; +import { parseGitHubContext, isEntityContext } from "../github/context"; +import { fetchPRBranchData } from "../github/data/pr-fetcher"; +import { createPrompt } from "../create-prompt"; +import { prepareMcpTools } from "../mcp/install-mcp-server"; +import { generateCombinePrompt } from "../create-prompt/templates/combine-prompt"; +import { normalizeDroidArgs, parseAllowedTools } from "../utils/parse-tools"; + +async function run() { + try { + const githubToken = process.env.GITHUB_TOKEN!; + const commentId = parseInt(process.env.DROID_COMMENT_ID || "0"); + const codeReviewResults = process.env.CODE_REVIEW_RESULTS || ""; + const securityResults = process.env.SECURITY_RESULTS || ""; + + const context = parseGitHubContext(); + + if (!isEntityContext(context)) { + throw new Error("Combine requires entity context (PR or issue)"); + } + + if (!context.isPR) { + throw new Error("Combine is only supported on pull requests"); + } + + const octokit = createOctokit(githubToken); + + const prData = await fetchPRBranchData({ + octokits: octokit, + repository: context.repository, + prNumber: context.entityNumber, + }); + + // Generate combine prompt with paths to result files + await createPrompt({ + githubContext: context, + commentId, + baseBranch: prData.baseRefName, + prBranchData: { + headRefName: prData.headRefName, + headRefOid: prData.headRefOid, + }, + generatePrompt: (ctx) => + generateCombinePrompt(ctx, codeReviewResults, securityResults), + }); + + core.exportVariable("DROID_EXEC_RUN_TYPE", "droid-combine"); + + const rawUserArgs = process.env.DROID_ARGS || ""; + const normalizedUserArgs = normalizeDroidArgs(rawUserArgs); + const userAllowedMCPTools = parseAllowedTools(normalizedUserArgs).filter( + (tool) => tool.startsWith("github_") && tool.includes("___"), + ); + + // Combine step has tools for inline comments and tracking comment update + // NO github_pr___submit_review - it creates duplicate summary comments + const baseTools = [ + "Read", + "Grep", + "Glob", + "LS", + "Execute", + "github_comment___update_droid_comment", + "github_inline_comment___create_inline_comment", + ]; + + const allowedTools = Array.from( + new Set([...baseTools, ...userAllowedMCPTools]), + ); + + const mcpTools = await prepareMcpTools({ + githubToken, + owner: context.repository.owner, + repo: context.repository.repo, + droidCommentId: commentId.toString(), + allowedTools, + mode: "tag", + context, + }); + + const droidArgParts: string[] = []; + // Only include built-in tools in --enabled-tools + const builtInTools = allowedTools.filter((t) => !t.includes("___")); + if (builtInTools.length > 0) { + droidArgParts.push(`--enabled-tools "${builtInTools.join(",")}"`); + } + + if (normalizedUserArgs) { + droidArgParts.push(normalizedUserArgs); + } + + core.setOutput("droid_args", droidArgParts.join(" ").trim()); + core.setOutput("mcp_tools", mcpTools); + + console.log(`Generated combine prompt`); + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + core.setFailed(`Generate combine prompt failed: ${errorMessage}`); + process.exit(1); + } +} + +if (import.meta.main) { + run(); +} diff --git a/src/entrypoints/generate-review-prompt.ts b/src/entrypoints/generate-review-prompt.ts new file mode 100644 index 0000000..c52d706 --- /dev/null +++ b/src/entrypoints/generate-review-prompt.ts @@ -0,0 +1,138 @@ +#!/usr/bin/env bun + +/** + * Generate review prompt for standalone review/security actions + */ + +import * as core from "@actions/core"; +import { createOctokit } from "../github/api/client"; +import { parseGitHubContext, isEntityContext } from "../github/context"; +import { fetchPRBranchData } from "../github/data/pr-fetcher"; +import { createPrompt } from "../create-prompt"; +import { prepareMcpTools } from "../mcp/install-mcp-server"; +import { generateReviewPrompt } from "../create-prompt/templates/review-prompt"; +import { generateSecurityReviewPrompt } from "../create-prompt/templates/security-review-prompt"; +import { normalizeDroidArgs, parseAllowedTools } from "../utils/parse-tools"; + +async function run() { + try { + const githubToken = process.env.GITHUB_TOKEN!; + const reviewType = process.env.REVIEW_TYPE || "code"; + const commentId = parseInt(process.env.DROID_COMMENT_ID || "0"); + + const context = parseGitHubContext(); + + if (!isEntityContext(context)) { + throw new Error("Review requires entity context (PR or issue)"); + } + + if (!context.isPR) { + throw new Error("Review is only supported on pull requests"); + } + + const octokit = createOctokit(githubToken); + + const prData = await fetchPRBranchData({ + octokits: octokit, + repository: context.repository, + prNumber: context.entityNumber, + }); + + const branchInfo = { + baseBranch: prData.baseRefName, + currentBranch: prData.headRefName, + }; + + // Select prompt generator based on review type + const generatePrompt = + reviewType === "security" + ? generateSecurityReviewPrompt + : generateReviewPrompt; + + await createPrompt({ + githubContext: context, + commentId, + baseBranch: branchInfo.baseBranch, + prBranchData: { + headRefName: prData.headRefName, + headRefOid: prData.headRefOid, + }, + generatePrompt, + }); + + // Set run type + const runType = + reviewType === "security" ? "droid-security-review" : "droid-review"; + core.exportVariable("DROID_EXEC_RUN_TYPE", runType); + + const rawUserArgs = process.env.DROID_ARGS || ""; + const normalizedUserArgs = normalizeDroidArgs(rawUserArgs); + const userAllowedMCPTools = parseAllowedTools(normalizedUserArgs).filter( + (tool) => tool.startsWith("github_") && tool.includes("___"), + ); + + // Base tools for analysis only - NO inline comment tools + // Inline comments will be posted by the finalize step to avoid overlaps + const baseTools = [ + "Read", + "Grep", + "Glob", + "LS", + "Execute", + "github_comment___update_droid_comment", + ]; + + // Review tools for reading existing comments only + const reviewTools = ["github_pr___list_review_comments"]; + + const allowedTools = Array.from( + new Set([...baseTools, ...reviewTools, ...userAllowedMCPTools]), + ); + + const mcpTools = await prepareMcpTools({ + githubToken, + owner: context.repository.owner, + repo: context.repository.repo, + droidCommentId: commentId.toString(), + allowedTools, + mode: "tag", + context, + }); + + const droidArgParts: string[] = []; + // Only include built-in tools in --enabled-tools + // MCP tools are discovered dynamically from registered servers + const builtInTools = allowedTools.filter((t) => !t.includes("___")); + if (builtInTools.length > 0) { + droidArgParts.push(`--enabled-tools "${builtInTools.join(",")}"`); + } + + // Add model override if specified + const model = + reviewType === "security" + ? process.env.SECURITY_MODEL?.trim() || process.env.REVIEW_MODEL?.trim() + : process.env.REVIEW_MODEL?.trim(); + + if (model) { + droidArgParts.push(`--model "${model}"`); + } + + if (normalizedUserArgs) { + droidArgParts.push(normalizedUserArgs); + } + + // Output for next step - use core.setOutput which handles GITHUB_OUTPUT internally + core.setOutput("droid_args", droidArgParts.join(" ").trim()); + core.setOutput("mcp_tools", mcpTools); + + console.log(`Generated ${reviewType} review prompt`); + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + core.setFailed(`Generate prompt failed: ${errorMessage}`); + process.exit(1); + } +} + +if (import.meta.main) { + run(); +} diff --git a/src/entrypoints/get-token.ts b/src/entrypoints/get-token.ts new file mode 100644 index 0000000..ca6c697 --- /dev/null +++ b/src/entrypoints/get-token.ts @@ -0,0 +1,40 @@ +#!/usr/bin/env bun + +/** + * Gets GitHub token via OIDC or uses provided token + */ + +import * as core from "@actions/core"; +import { appendFileSync } from "fs"; +import { setupGitHubToken } from "../github/token"; + +async function run() { + try { + const overrideToken = process.env.OVERRIDE_GITHUB_TOKEN?.trim(); + + let token: string; + if (overrideToken) { + console.log("Using provided GitHub token"); + token = overrideToken; + } else { + console.log("Requesting OIDC token..."); + token = await setupGitHubToken(); + console.log("GitHub token obtained via OIDC"); + } + + // Set output for next steps + const githubOutput = process.env.GITHUB_OUTPUT; + if (githubOutput) { + appendFileSync(githubOutput, `github_token=${token}\n`); + } + core.setOutput("github_token", token); + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + core.setFailed(`Failed to get GitHub token: ${errorMessage}`); + process.exit(1); + } +} + +if (import.meta.main) { + run(); +} diff --git a/src/entrypoints/prepare.ts b/src/entrypoints/prepare.ts index cb09303..5763f35 100644 --- a/src/entrypoints/prepare.ts +++ b/src/entrypoints/prepare.ts @@ -73,7 +73,6 @@ async function run() { if (result?.mcpTools) { core.setOutput("mcp_tools", result.mcpTools); } - } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); core.setFailed(`Prepare step failed with error: ${errorMessage}`); diff --git a/src/github/api/queries/github.ts b/src/github/api/queries/github.ts index 2341a55..36efc22 100644 --- a/src/github/api/queries/github.ts +++ b/src/github/api/queries/github.ts @@ -123,3 +123,13 @@ export const USER_QUERY = ` } } `; + +export const REPO_DEFAULT_BRANCH_QUERY = ` + query($owner: String!, $repo: String!) { + repository(owner: $owner, name: $repo) { + defaultBranchRef { + name + } + } + } +`; diff --git a/src/github/context.ts b/src/github/context.ts index 9b4306d..ae52994 100644 --- a/src/github/context.ts +++ b/src/github/context.ts @@ -90,6 +90,14 @@ type BaseContext = { allowedNonWriteUsers: string; trackProgress: boolean; automaticReview: boolean; + automaticSecurityReview: boolean; + securityModel: string; + securitySeverityThreshold: string; + securityBlockOnCritical: boolean; + securityBlockOnHigh: boolean; + securityNotifyTeam: string; + securityScanSchedule: boolean; + securityScanDays: number; }; }; @@ -140,6 +148,16 @@ export function parseGitHubContext(): GitHubContext { allowedNonWriteUsers: process.env.ALLOWED_NON_WRITE_USERS ?? "", trackProgress: process.env.TRACK_PROGRESS === "true", automaticReview: process.env.AUTOMATIC_REVIEW === "true", + automaticSecurityReview: process.env.AUTOMATIC_SECURITY_REVIEW === "true", + securityModel: process.env.SECURITY_MODEL ?? "", + securitySeverityThreshold: + process.env.SECURITY_SEVERITY_THRESHOLD ?? "medium", + securityBlockOnCritical: + process.env.SECURITY_BLOCK_ON_CRITICAL !== "false", + securityBlockOnHigh: process.env.SECURITY_BLOCK_ON_HIGH === "true", + securityNotifyTeam: process.env.SECURITY_NOTIFY_TEAM ?? "", + securityScanSchedule: process.env.SECURITY_SCAN_SCHEDULE === "true", + securityScanDays: parseInt(process.env.SECURITY_SCAN_DAYS ?? "7", 10), }, }; diff --git a/src/github/data/pr-fetcher.ts b/src/github/data/pr-fetcher.ts index 6cc04fb..28e7188 100644 --- a/src/github/data/pr-fetcher.ts +++ b/src/github/data/pr-fetcher.ts @@ -1,5 +1,5 @@ import type { Octokits } from "../api/client"; -import { PR_QUERY } from "../api/queries/github"; +import { PR_QUERY, REPO_DEFAULT_BRANCH_QUERY } from "../api/queries/github"; import type { GitHubPullRequest } from "../types"; /** @@ -58,3 +58,46 @@ export async function fetchPRBranchData({ throw new Error(`Failed to fetch PR branch data for PR #${prNumber}`); } } + +type RepoDefaultBranchQueryResponse = { + repository: { + defaultBranchRef: { + name: string; + } | null; + }; +}; + +/** + * Fetches the repository's default branch name. + * Used by security-scan which operates without a PR context. + */ +export async function fetchRepoDefaultBranch({ + octokits, + repository, +}: { + octokits: Octokits; + repository: { owner: string; repo: string }; +}): Promise { + try { + const result = await octokits.graphql( + REPO_DEFAULT_BRANCH_QUERY, + { + owner: repository.owner, + repo: repository.repo, + }, + ); + + if (!result.repository.defaultBranchRef) { + throw new Error( + `Default branch not found for ${repository.owner}/${repository.repo}`, + ); + } + + return result.repository.defaultBranchRef.name; + } catch (error) { + console.error(`Failed to fetch default branch:`, error); + throw new Error( + `Failed to fetch default branch for ${repository.owner}/${repository.repo}`, + ); + } +} diff --git a/src/github/operations/comments/common.ts b/src/github/operations/comments/common.ts index 0ff5429..d3ab2be 100644 --- a/src/github/operations/comments/common.ts +++ b/src/github/operations/comments/common.ts @@ -18,11 +18,19 @@ export function createBranchLink( return `\n[View branch](${branchUrl})`; } +export type CommentType = "default" | "security"; + export function createCommentBody( jobRunLink: string, branchLink: string = "", + type: CommentType = "default", ): string { - return `Droid is working… + const message = + type === "security" + ? "Droid is running a security check…" + : "Droid is working…"; + + return `${message} ${jobRunLink}${branchLink}`; } diff --git a/src/github/operations/comments/create-initial.ts b/src/github/operations/comments/create-initial.ts index 46f2047..2292fb8 100644 --- a/src/github/operations/comments/create-initial.ts +++ b/src/github/operations/comments/create-initial.ts @@ -6,7 +6,11 @@ */ import { appendFileSync } from "fs"; -import { createJobRunLink, createCommentBody } from "./common"; +import { + createJobRunLink, + createCommentBody, + type CommentType, +} from "./common"; import { isPullRequestReviewCommentEvent, isPullRequestEvent, @@ -19,11 +23,12 @@ const DROID_APP_BOT_ID = 209825114; export async function createInitialComment( octokit: Octokit, context: ParsedGitHubContext, + commentType: CommentType = "default", ) { const { owner, repo } = context.repository; const jobRunLink = createJobRunLink(owner, repo, context.runId); - const initialBody = createCommentBody(jobRunLink); + const initialBody = createCommentBody(jobRunLink, "", commentType); try { let response; diff --git a/src/github/token.ts b/src/github/token.ts index 986f0b6..471f8ba 100644 --- a/src/github/token.ts +++ b/src/github/token.ts @@ -41,45 +41,57 @@ async function exchangeForAppToken(oidcToken: string): Promise { // Handle the simplified flat error response format const errorCode = responseJson.error || `http_${response.status}`; - const errorMessage = responseJson.message || responseJson.detail || responseJson.error || "Unknown error"; + const errorMessage = + responseJson.message || + responseJson.detail || + responseJson.error || + "Unknown error"; const specificErrorCode = responseJson.error_code; const repository = responseJson.repository; // Check for specific error codes that should skip the action - if (errorCode === "workflow_validation_failed" || - specificErrorCode === "workflow_not_found_on_default_branch") { - core.warning(`Skipping action due to workflow validation: ${errorMessage}`); + if ( + errorCode === "workflow_validation_failed" || + specificErrorCode === "workflow_not_found_on_default_branch" + ) { + core.warning( + `Skipping action due to workflow validation: ${errorMessage}`, + ); console.log( "Action skipped due to workflow validation error. This is expected when adding Droid workflows to new repositories or on PRs with workflow changes. If you're seeing this, your workflow will begin working once you merge your PR.", ); core.setOutput("skipped_due_to_workflow_validation_mismatch", "true"); process.exit(0); } - + // Handle GitHub App not installed error with helpful message if (errorCode === "app_not_installed") { const repo = repository || "this repository"; console.error( `The Factory GitHub App is not installed for ${repo}. ` + - `Please install it at: https://github.com/apps/factory-ai` + `Please install it at: https://github.com/apps/factory-ai`, ); throw new Error(errorMessage); } - + // Handle rate limiting with retry suggestion if (errorCode === "rate_limited") { console.error( - `GitHub API rate limit exceeded. Please wait a few minutes and try again.` + `GitHub API rate limit exceeded. Please wait a few minutes and try again.`, ); throw new Error(errorMessage); } - + // Handle OIDC verification errors if (errorCode === "oidc_verification_failed") { if (specificErrorCode === "token_expired") { - console.error("OIDC token has expired. The workflow may be taking too long."); + console.error( + "OIDC token has expired. The workflow may be taking too long.", + ); } else if (specificErrorCode === "audience_mismatch") { - console.error("OIDC token audience mismatch. This is likely a configuration issue."); + console.error( + "OIDC token audience mismatch. This is likely a configuration issue.", + ); } else if (specificErrorCode === "invalid_signature") { console.error("OIDC token signature verification failed."); } @@ -89,7 +101,7 @@ async function exchangeForAppToken(oidcToken: string): Promise { console.error( `App token exchange failed: ${response.status} ${response.statusText} - ${errorMessage}`, errorCode !== errorMessage ? `(Code: ${errorCode})` : "", - specificErrorCode ? `(Specific: ${specificErrorCode})` : "" + specificErrorCode ? `(Specific: ${specificErrorCode})` : "", ); throw new Error(errorMessage); } @@ -99,7 +111,7 @@ async function exchangeForAppToken(oidcToken: string): Promise { token?: string; expires_at?: string; }; - + if (!appTokenData.token) { throw new Error("App token not found in response"); } diff --git a/src/github/utils/command-parser.test.ts b/src/github/utils/command-parser.test.ts index b0be23f..602a13b 100644 --- a/src/github/utils/command-parser.test.ts +++ b/src/github/utils/command-parser.test.ts @@ -27,6 +27,14 @@ const baseContext: Omit = { allowedNonWriteUsers: "", trackProgress: false, automaticReview: false, + automaticSecurityReview: false, + securityModel: "", + securitySeverityThreshold: "medium", + securityBlockOnCritical: true, + securityBlockOnHigh: false, + securityNotifyTeam: "", + securityScanSchedule: false, + securityScanDays: 7, }, entityNumber: 1, isPR: true, @@ -71,6 +79,61 @@ describe("Command Parser", () => { expect(result?.raw).toBe("@droid review"); }); + it("should detect @droid review security (combined)", () => { + const result = parseDroidCommand("@droid review security"); + expect(result?.command).toBe("review-security"); + expect(result?.raw).toBe("@droid review security"); + }); + + it("should detect @droid security review (combined)", () => { + const result = parseDroidCommand("@droid security review"); + expect(result?.command).toBe("review-security"); + expect(result?.raw).toBe("@droid security review"); + }); + + it("should be case insensitive for combined commands", () => { + const result = parseDroidCommand("@DROID REVIEW SECURITY"); + expect(result?.command).toBe("review-security"); + }); + + it("should prioritize combined over individual review", () => { + const result = parseDroidCommand("@droid review security please"); + expect(result?.command).toBe("review-security"); + }); + + it("should detect @droid security", () => { + const result = parseDroidCommand("Please @droid security this PR"); + expect(result?.command).toBe("security"); + expect(result?.raw).toBe("@droid security"); + }); + + it("should detect @droid security at end of text", () => { + const result = parseDroidCommand("Please run @droid security"); + expect(result?.command).toBe("security"); + expect(result?.raw).toBe("@droid security"); + }); + + it("should be case insensitive for @droid security", () => { + const result = parseDroidCommand("@DROID SECURITY"); + expect(result?.command).toBe("security"); + }); + + it("should detect @droid security --full", () => { + const result = parseDroidCommand("@droid security --full"); + expect(result?.command).toBe("security-full"); + expect(result?.raw).toBe("@droid security --full"); + }); + + it("should be case insensitive for @droid security --full", () => { + const result = parseDroidCommand("@DROID SECURITY --FULL"); + expect(result?.command).toBe("security-full"); + }); + + it("should prioritize security-full over security", () => { + const result = parseDroidCommand("@droid security --full this repo"); + expect(result?.command).toBe("security-full"); + }); + it("should prioritize specific commands over default", () => { // If text has both @droid fill and just @droid, should detect fill const result = parseDroidCommand("@droid please @droid fill this"); @@ -125,17 +188,14 @@ describe("Command Parser", () => { describe("extractCommandFromContext", () => { it("should extract from PR body", () => { - const context = createContext( - "pull_request", - { - action: "opened", - pull_request: { - body: "PR description\n\n@droid fill", - number: 1, - title: "PR", - }, - } as unknown as PullRequestEvent, - ); + const context = createContext("pull_request", { + action: "opened", + pull_request: { + body: "PR description\n\n@droid fill", + number: 1, + title: "PR", + }, + } as unknown as PullRequestEvent); const result = extractCommandFromContext(context); expect(result?.command).toBe("fill"); expect(result?.location).toBe("body"); @@ -160,20 +220,17 @@ describe("Command Parser", () => { }); it("should extract from issue comment", () => { - const context = createContext( - "issue_comment", - { - action: "created", - comment: { - body: "@droid fill please", - created_at: "2024-01-01T00:00:00Z", - }, - issue: { - number: 1, - pull_request: { url: "" }, - }, - } as unknown as IssueCommentEvent, - ); + const context = createContext("issue_comment", { + action: "created", + comment: { + body: "@droid fill please", + created_at: "2024-01-01T00:00:00Z", + }, + issue: { + number: 1, + pull_request: { url: "" }, + }, + } as unknown as IssueCommentEvent); const result = extractCommandFromContext(context); expect(result?.command).toBe("fill"); expect(result?.location).toBe("comment"); @@ -181,19 +238,16 @@ describe("Command Parser", () => { }); it("should extract from PR review comment", () => { - const context = createContext( - "pull_request_review_comment", - { - action: "created", - comment: { - body: "Can you @droid review this section?", - created_at: "2024-01-01T00:00:00Z", - }, - pull_request: { - number: 1, - }, - } as unknown as PullRequestReviewCommentEvent, - ); + const context = createContext("pull_request_review_comment", { + action: "created", + comment: { + body: "Can you @droid review this section?", + created_at: "2024-01-01T00:00:00Z", + }, + pull_request: { + number: 1, + }, + } as unknown as PullRequestReviewCommentEvent); const result = extractCommandFromContext(context); expect(result?.command).toBe("review"); expect(result?.location).toBe("comment"); @@ -201,37 +255,62 @@ describe("Command Parser", () => { }); it("should extract from PR review body", () => { - const context = createContext( - "pull_request_review", - { - action: "submitted", - review: { - body: "LGTM but @droid fill the description", - submitted_at: "2024-01-01T00:00:00Z", - }, - pull_request: { - number: 1, - }, - } as unknown as PullRequestReviewEvent, - ); + const context = createContext("pull_request_review", { + action: "submitted", + review: { + body: "LGTM but @droid fill the description", + submitted_at: "2024-01-01T00:00:00Z", + }, + pull_request: { + number: 1, + }, + } as unknown as PullRequestReviewEvent); const result = extractCommandFromContext(context); expect(result?.command).toBe("fill"); expect(result?.location).toBe("comment"); expect(result?.timestamp).toBe("2024-01-01T00:00:00Z"); }); + it("should extract security from PR body", () => { + const context = createContext("pull_request", { + action: "opened", + pull_request: { + body: "PR description\n\n@droid security", + number: 1, + title: "PR", + }, + } as unknown as PullRequestEvent); + const result = extractCommandFromContext(context); + expect(result?.command).toBe("security"); + expect(result?.location).toBe("body"); + }); + + it("should extract security-full from issue comment", () => { + const context = createContext("issue_comment", { + action: "created", + comment: { + body: "@droid security --full", + created_at: "2024-01-01T00:00:00Z", + }, + issue: { + number: 1, + pull_request: { url: "" }, + }, + } as unknown as IssueCommentEvent); + const result = extractCommandFromContext(context); + expect(result?.command).toBe("security-full"); + expect(result?.location).toBe("comment"); + }); + it("should return null for events without commands", () => { - const context = createContext( - "pull_request", - { - action: "opened", - pull_request: { - body: "Regular PR description", - number: 1, - title: "PR", - }, - } as unknown as PullRequestEvent, - ); + const context = createContext("pull_request", { + action: "opened", + pull_request: { + body: "Regular PR description", + number: 1, + title: "PR", + }, + } as unknown as PullRequestEvent); const result = extractCommandFromContext(context); expect(result).toBeNull(); }); @@ -247,17 +326,14 @@ describe("Command Parser", () => { }); it("should handle missing body gracefully", () => { - const context = createContext( - "pull_request", - { - action: "opened", - pull_request: { - body: null, - number: 1, - title: "PR", - }, - } as unknown as PullRequestEvent, - ); + const context = createContext("pull_request", { + action: "opened", + pull_request: { + body: null, + number: 1, + title: "PR", + }, + } as unknown as PullRequestEvent); const result = extractCommandFromContext(context); expect(result).toBeNull(); }); @@ -273,20 +349,17 @@ describe("Command Parser", () => { }); it("should extract default command when no specific command", () => { - const context = createContext( - "issue_comment", - { - action: "created", - comment: { - body: "@droid can you help with this?", - created_at: "2024-01-01T00:00:00Z", - }, - issue: { - number: 1, - pull_request: { url: "" }, - }, - } as unknown as IssueCommentEvent, - ); + const context = createContext("issue_comment", { + action: "created", + comment: { + body: "@droid can you help with this?", + created_at: "2024-01-01T00:00:00Z", + }, + issue: { + number: 1, + pull_request: { url: "" }, + }, + } as unknown as IssueCommentEvent); const result = extractCommandFromContext(context); expect(result?.command).toBe("default"); expect(result?.location).toBe("comment"); diff --git a/src/github/utils/command-parser.ts b/src/github/utils/command-parser.ts index a1250ca..182cac5 100644 --- a/src/github/utils/command-parser.ts +++ b/src/github/utils/command-parser.ts @@ -4,12 +4,18 @@ import type { GitHubContext } from "../context"; -export type DroidCommand = 'fill' | 'review' | 'default'; +export type DroidCommand = + | "fill" + | "review" + | "security" + | "review-security" + | "security-full" + | "default"; export interface ParsedCommand { command: DroidCommand; raw: string; - location: 'body' | 'comment'; + location: "body" | "comment"; timestamp?: string | null; } @@ -27,9 +33,22 @@ export function parseDroidCommand(text: string): ParsedCommand | null { const fillMatch = text.match(/@droid\s+fill/i); if (fillMatch) { return { - command: 'fill', + command: "fill", raw: fillMatch[0], - location: 'body', // Will be set by caller + location: "body", // Will be set by caller + }; + } + + // Check for @droid review security OR @droid security review (both reviews) + // Must check before individual review/security to avoid false matches + const combinedMatch = text.match( + /@droid\s+(?:review\s+security|security\s+review)/i, + ); + if (combinedMatch) { + return { + command: "review-security", + raw: combinedMatch[0], + location: "body", // Will be set by caller }; } @@ -37,9 +56,30 @@ export function parseDroidCommand(text: string): ParsedCommand | null { const reviewMatch = text.match(/@droid\s+review/i); if (reviewMatch) { return { - command: 'review', + command: "review", raw: reviewMatch[0], - location: 'body', // Will be set by caller + location: "body", // Will be set by caller + }; + } + + // Check for @droid security --full command (case insensitive) + const securityFullMatch = text.match(/@droid\s+security\s+--full/i); + if (securityFullMatch) { + return { + command: "security-full", + raw: securityFullMatch[0], + location: "body", // Will be set by caller + }; + } + + // Check for @droid security command (case insensitive) + // Must check after security-full to avoid false matches + const securityMatch = text.match(/@droid\s+security(?:\s|$|[^-\w])/i); + if (securityMatch) { + return { + command: "security", + raw: securityMatch[0].trim(), + location: "body", // Will be set by caller }; } @@ -47,9 +87,9 @@ export function parseDroidCommand(text: string): ParsedCommand | null { const droidMatch = text.match(/@droid/i); if (droidMatch) { return { - command: 'default', + command: "default", raw: droidMatch[0], - location: 'body', // Will be set by caller + location: "body", // Will be set by caller }; } @@ -61,43 +101,48 @@ export function parseDroidCommand(text: string): ParsedCommand | null { * @param context The GitHub context from the event * @returns ParsedCommand with location info, or null if no command found */ -export function extractCommandFromContext(context: GitHubContext): ParsedCommand | null { +export function extractCommandFromContext( + context: GitHubContext, +): ParsedCommand | null { // Handle missing payload if (!context.payload) { return null; } // Check PR body for commands (pull_request events) - if (context.eventName === 'pull_request' && 'pull_request' in context.payload) { + if ( + context.eventName === "pull_request" && + "pull_request" in context.payload + ) { const body = context.payload.pull_request.body; if (body) { const command = parseDroidCommand(body); if (command) { - return { ...command, location: 'body' }; + return { ...command, location: "body" }; } } } // Check issue body for commands (issues events) - if (context.eventName === 'issues' && 'issue' in context.payload) { + if (context.eventName === "issues" && "issue" in context.payload) { const body = context.payload.issue.body; if (body) { const command = parseDroidCommand(body); if (command) { - return { ...command, location: 'body' }; + return { ...command, location: "body" }; } } } // Check comment body for commands (issue_comment events) - if (context.eventName === 'issue_comment' && 'comment' in context.payload) { + if (context.eventName === "issue_comment" && "comment" in context.payload) { const comment = context.payload.comment; if (comment.body) { const command = parseDroidCommand(comment.body); if (command) { return { ...command, - location: 'comment', + location: "comment", timestamp: comment.created_at, }; } @@ -105,14 +150,17 @@ export function extractCommandFromContext(context: GitHubContext): ParsedCommand } // Check review comment body (pull_request_review_comment events) - if (context.eventName === 'pull_request_review_comment' && 'comment' in context.payload) { + if ( + context.eventName === "pull_request_review_comment" && + "comment" in context.payload + ) { const comment = context.payload.comment; if (comment.body) { const command = parseDroidCommand(comment.body); if (command) { return { ...command, - location: 'comment', + location: "comment", timestamp: comment.created_at, }; } @@ -120,14 +168,17 @@ export function extractCommandFromContext(context: GitHubContext): ParsedCommand } // Check review body (pull_request_review events) - if (context.eventName === 'pull_request_review' && 'review' in context.payload) { + if ( + context.eventName === "pull_request_review" && + "review" in context.payload + ) { const review = context.payload.review; if (review.body) { const command = parseDroidCommand(review.body); if (command) { return { ...command, - location: 'comment', + location: "comment", timestamp: review.submitted_at, }; } diff --git a/src/github/validation/trigger-commands.test.ts b/src/github/validation/trigger-commands.test.ts index 1d44a46..aa0197d 100644 --- a/src/github/validation/trigger-commands.test.ts +++ b/src/github/validation/trigger-commands.test.ts @@ -9,8 +9,9 @@ import type { PullRequestReviewEvent, } from "@octokit/webhooks-types"; -type ContextOverrides = - Partial> & { payload?: unknown }; +type ContextOverrides = Partial> & { + payload?: unknown; +}; const defaultPayload = { action: "created", @@ -27,7 +28,9 @@ const defaultPayload = { } as unknown as IssueCommentEvent; // Helper function to create a mock context -function createMockContext(overrides: ContextOverrides = {}): ParsedGitHubContext { +function createMockContext( + overrides: ContextOverrides = {}, +): ParsedGitHubContext { return { runId: "run-1", eventName: "issue_comment", @@ -50,6 +53,7 @@ function createMockContext(overrides: ContextOverrides = {}): ParsedGitHubContex botName: "droid[bot]", trackProgress: false, automaticReview: false, + automaticSecurityReview: false, useStickyComment: false, }, payload: defaultPayload, @@ -70,7 +74,7 @@ describe("checkContainsTrigger with commands", () => { }, } as unknown as PullRequestEvent, }); - + expect(checkContainsTrigger(context)).toBe(true); }); @@ -84,7 +88,7 @@ describe("checkContainsTrigger with commands", () => { }, } as unknown as IssueCommentEvent, }); - + expect(checkContainsTrigger(context)).toBe(true); }); @@ -101,7 +105,7 @@ describe("checkContainsTrigger with commands", () => { }, } as unknown as PullRequestReviewCommentEvent, }); - + expect(checkContainsTrigger(context)).toBe(true); }); @@ -118,7 +122,21 @@ describe("checkContainsTrigger with commands", () => { }, } as unknown as PullRequestReviewEvent, }); - + + expect(checkContainsTrigger(context)).toBe(true); + }); + + it("should trigger on @droid security in issue comment", () => { + const context = createMockContext({ + eventName: "issue_comment", + eventAction: "created", + payload: { + comment: { + body: "Can you @droid security this PR?", + }, + } as unknown as IssueCommentEvent, + }); + expect(checkContainsTrigger(context)).toBe(true); }); @@ -132,7 +150,7 @@ describe("checkContainsTrigger with commands", () => { }, } as unknown as IssueCommentEvent, }); - + // This should still trigger because of the existing trigger phrase logic // but now it will be handled as default command expect(checkContainsTrigger(context)).toBe(true); @@ -148,7 +166,7 @@ describe("checkContainsTrigger with commands", () => { }, } as unknown as IssueCommentEvent, }); - + expect(checkContainsTrigger(context)).toBe(true); }); @@ -162,7 +180,7 @@ describe("checkContainsTrigger with commands", () => { }, } as unknown as IssueCommentEvent, }); - + expect(checkContainsTrigger(context)).toBe(false); }); @@ -179,7 +197,7 @@ describe("checkContainsTrigger with commands", () => { }, } as unknown as IssuesEvent, }); - + expect(checkContainsTrigger(context)).toBe(true); }); }); diff --git a/src/github/validation/trigger.ts b/src/github/validation/trigger.ts index 51e8d25..a39ce0d 100644 --- a/src/github/validation/trigger.ts +++ b/src/github/validation/trigger.ts @@ -18,8 +18,10 @@ export function checkContainsTrigger(context: ParsedGitHubContext): boolean { // Check for specific @droid commands (fill, review) const command = extractCommandFromContext(context); - if (command && ['fill', 'review'].includes(command.command)) { - console.log(`Detected @droid ${command.command} command, triggering action`); + if (command && ["fill", "review", "security"].includes(command.command)) { + console.log( + `Detected @droid ${command.command} command, triggering action`, + ); return true; } diff --git a/src/mcp/github-inline-comment-server.ts b/src/mcp/github-inline-comment-server.ts index 3b9ae31..2a51185 100644 --- a/src/mcp/github-inline-comment-server.ts +++ b/src/mcp/github-inline-comment-server.ts @@ -59,9 +59,9 @@ server.tool( .optional() .default("RIGHT") .describe( - "Side of the diff to comment on: LEFT (old code) or RIGHT (new code). " + - "IMPORTANT: Use RIGHT for comments on new/modified code. " + - "Only use LEFT when specifically referencing code being removed." + "Side of the diff to comment on: LEFT (old code) or RIGHT (new code). " + + "IMPORTANT: Use RIGHT for comments on new/modified code. " + + "Only use LEFT when specifically referencing code being removed.", ), commit_id: z .string() diff --git a/src/mcp/install-mcp-server.ts b/src/mcp/install-mcp-server.ts index 44b5c46..0714523 100644 --- a/src/mcp/install-mcp-server.ts +++ b/src/mcp/install-mcp-server.ts @@ -93,7 +93,6 @@ export async function prepareMcpTools( }, }; - // Include inline comment server for PRs when requested via allowed tools if ( isEntityContext(context) && @@ -118,9 +117,7 @@ export async function prepareMcpTools( const hasWorkflowToken = !!process.env.DEFAULT_WORKFLOW_TOKEN; const shouldIncludeCIServer = - isEntityContext(context) && - context.isPR && - hasWorkflowToken; + isEntityContext(context) && context.isPR && hasWorkflowToken; if (shouldIncludeCIServer) { // Verify the token actually has actions:read permission diff --git a/src/prepare/types.ts b/src/prepare/types.ts index 0235a20..2276ec2 100644 --- a/src/prepare/types.ts +++ b/src/prepare/types.ts @@ -9,6 +9,8 @@ export type PrepareResult = { currentBranch: string; }; mcpTools: string; + skipped?: boolean; + reason?: string; }; export type PrepareOptions = { diff --git a/src/tag/commands/fill.ts b/src/tag/commands/fill.ts index 67a5e19..1cc7e31 100644 --- a/src/tag/commands/fill.ts +++ b/src/tag/commands/fill.ts @@ -33,7 +33,7 @@ export async function prepareFillMode({ const commentId = trackingCommentId ?? (await createInitialComment(octokit.rest, context)).id; - + const prData = await fetchPRBranchData({ octokits: octokit, repository: context.repository, diff --git a/src/tag/commands/security-review.ts b/src/tag/commands/security-review.ts new file mode 100644 index 0000000..4033217 --- /dev/null +++ b/src/tag/commands/security-review.ts @@ -0,0 +1,128 @@ +import * as core from "@actions/core"; +import type { GitHubContext } from "../../github/context"; +import { fetchPRBranchData } from "../../github/data/pr-fetcher"; +import { createPrompt } from "../../create-prompt"; +import { prepareMcpTools } from "../../mcp/install-mcp-server"; +import { createInitialComment } from "../../github/operations/comments/create-initial"; +import { normalizeDroidArgs, parseAllowedTools } from "../../utils/parse-tools"; +import { isEntityContext } from "../../github/context"; +import { generateSecurityReviewPrompt } from "../../create-prompt/templates/security-review-prompt"; +import type { Octokits } from "../../github/api/client"; +import type { PrepareResult } from "../../prepare/types"; + +type SecurityReviewCommandOptions = { + context: GitHubContext; + octokit: Octokits; + githubToken: string; + trackingCommentId?: number; +}; + +export async function prepareSecurityReviewMode({ + context, + octokit, + githubToken, + trackingCommentId, +}: SecurityReviewCommandOptions): Promise { + if (!isEntityContext(context)) { + throw new Error("Security review command requires an entity event context"); + } + + if (!context.isPR) { + throw new Error( + "Security review command is only supported on pull requests", + ); + } + + const commentId = + trackingCommentId ?? (await createInitialComment(octokit.rest, context)).id; + + const prData = await fetchPRBranchData({ + octokits: octokit, + repository: context.repository, + prNumber: context.entityNumber, + }); + + const branchInfo = { + baseBranch: prData.baseRefName, + droidBranch: undefined, + currentBranch: prData.headRefName, + }; + + await createPrompt({ + githubContext: context, + commentId, + baseBranch: branchInfo.baseBranch, + droidBranch: branchInfo.droidBranch, + prBranchData: { + headRefName: prData.headRefName, + headRefOid: prData.headRefOid, + }, + generatePrompt: generateSecurityReviewPrompt, + }); + core.exportVariable("DROID_EXEC_RUN_TYPE", "droid-security-review"); + + // Signal that security skills should be installed + core.setOutput("install_security_skills", "true"); + + const rawUserArgs = process.env.DROID_ARGS || ""; + const normalizedUserArgs = normalizeDroidArgs(rawUserArgs); + const userAllowedMCPTools = parseAllowedTools(normalizedUserArgs).filter( + (tool) => tool.startsWith("github_") && tool.includes("___"), + ); + + const baseTools = [ + "Read", + "Grep", + "Glob", + "LS", + "Execute", + "github_comment___update_droid_comment", + "github_inline_comment___create_inline_comment", + ]; + + const reviewTools = [ + "github_pr___list_review_comments", + "github_pr___submit_review", + "github_pr___delete_comment", + "github_pr___minimize_comment", + "github_pr___reply_to_comment", + "github_pr___resolve_review_thread", + ]; + + const allowedTools = Array.from( + new Set([...baseTools, ...reviewTools, ...userAllowedMCPTools]), + ); + + const mcpTools = await prepareMcpTools({ + githubToken, + owner: context.repository.owner, + repo: context.repository.repo, + droidCommentId: commentId.toString(), + allowedTools, + mode: "tag", + context, + }); + + const droidArgParts: string[] = []; + droidArgParts.push(`--enabled-tools "${allowedTools.join(",")}"`); + + // Add model override if specified (prefer SECURITY_MODEL, fallback to REVIEW_MODEL) + const securityModel = + process.env.SECURITY_MODEL?.trim() || process.env.REVIEW_MODEL?.trim(); + if (securityModel) { + droidArgParts.push(`--model "${securityModel}"`); + } + + if (normalizedUserArgs) { + droidArgParts.push(normalizedUserArgs); + } + + core.setOutput("droid_args", droidArgParts.join(" ").trim()); + core.setOutput("mcp_tools", mcpTools); + + return { + commentId, + branchInfo, + mcpTools, + }; +} diff --git a/src/tag/commands/security-scan.ts b/src/tag/commands/security-scan.ts new file mode 100644 index 0000000..5f474fc --- /dev/null +++ b/src/tag/commands/security-scan.ts @@ -0,0 +1,107 @@ +import * as core from "@actions/core"; +import type { GitHubContext } from "../../github/context"; +import { fetchRepoDefaultBranch } from "../../github/data/pr-fetcher"; +import { createPrompt } from "../../create-prompt"; +import { prepareMcpTools } from "../../mcp/install-mcp-server"; +import { normalizeDroidArgs, parseAllowedTools } from "../../utils/parse-tools"; +import { isEntityContext } from "../../github/context"; +import { generateSecurityReportPrompt } from "../../create-prompt/templates/security-report-prompt"; +import type { Octokits } from "../../github/api/client"; +import type { PrepareResult } from "../../prepare/types"; + +export type ScanScope = { type: "full" } | { type: "scheduled"; days: number }; + +type SecurityScanCommandOptions = { + context: GitHubContext; + octokit: Octokits; + githubToken: string; + scanScope: ScanScope; +}; + +export async function prepareSecurityScanMode({ + context, + octokit, + githubToken, + scanScope, +}: SecurityScanCommandOptions): Promise { + if (!isEntityContext(context)) { + throw new Error("Security scan command requires an entity event context"); + } + + // Fetch the repository's default branch (could be main, master, develop, etc.) + const defaultBranch = await fetchRepoDefaultBranch({ + octokits: octokit, + repository: context.repository, + }); + + const date = new Date().toISOString().split("T")[0]; + const branchName = `droid/security-report-${date}`; + + const branchInfo = { + baseBranch: defaultBranch, + droidBranch: branchName, + currentBranch: branchName, + }; + + await createPrompt({ + githubContext: context, + baseBranch: branchInfo.baseBranch, + droidBranch: branchInfo.droidBranch, + generatePrompt: (ctx) => + generateSecurityReportPrompt(ctx, scanScope, branchName), + }); + core.exportVariable("DROID_EXEC_RUN_TYPE", "droid-security-scan"); + + // Signal that security skills should be installed + core.setOutput("install_security_skills", "true"); + + const rawUserArgs = process.env.DROID_ARGS || ""; + const normalizedUserArgs = normalizeDroidArgs(rawUserArgs); + const userAllowedMCPTools = parseAllowedTools(normalizedUserArgs).filter( + (tool) => tool.startsWith("github_") && tool.includes("___"), + ); + + const baseTools = [ + "Read", + "Grep", + "Glob", + "LS", + "Execute", + "github_comment___update_droid_comment", + ]; + + const allowedTools = Array.from( + new Set([...baseTools, ...userAllowedMCPTools]), + ); + + const mcpTools = await prepareMcpTools({ + githubToken, + owner: context.repository.owner, + repo: context.repository.repo, + allowedTools, + mode: "tag", + context, + }); + + const droidArgParts: string[] = []; + droidArgParts.push(`--enabled-tools "${allowedTools.join(",")}"`); + + // Add model override if specified (prefer SECURITY_MODEL, fallback to REVIEW_MODEL) + const securityModel = + process.env.SECURITY_MODEL?.trim() || process.env.REVIEW_MODEL?.trim(); + if (securityModel) { + droidArgParts.push(`--model "${securityModel}"`); + } + + if (normalizedUserArgs) { + droidArgParts.push(normalizedUserArgs); + } + + core.setOutput("droid_args", droidArgParts.join(" ").trim()); + core.setOutput("mcp_tools", mcpTools); + + return { + branchInfo, + mcpTools, + }; +} diff --git a/src/tag/index.ts b/src/tag/index.ts index 7f036c1..a37d4b3 100644 --- a/src/tag/index.ts +++ b/src/tag/index.ts @@ -1,24 +1,66 @@ +import * as core from "@actions/core"; import { checkContainsTrigger } from "../github/validation/trigger"; import { checkHumanActor } from "../github/validation/actor"; import { createInitialComment } from "../github/operations/comments/create-initial"; -import { isEntityContext } from "../github/context"; +import { isEntityContext, type ParsedGitHubContext } from "../github/context"; import { extractCommandFromContext } from "../github/utils/command-parser"; import { prepareFillMode } from "./commands/fill"; import { prepareReviewMode } from "./commands/review"; +import { prepareSecurityReviewMode } from "./commands/security-review"; +import { prepareSecurityScanMode } from "./commands/security-scan"; import type { GitHubContext } from "../github/context"; import type { PrepareResult } from "../prepare/types"; import type { Octokits } from "../github/api/client"; +const DROID_APP_BOT_ID = 209825114; +const SECURITY_REVIEW_MARKER = "## Security Review Summary"; + export function shouldTriggerTag(context: GitHubContext): boolean { if (!isEntityContext(context)) { return false; } - if (context.inputs.automaticReview) { + if ( + context.inputs.automaticReview || + context.inputs.automaticSecurityReview + ) { return context.isPR; } return checkContainsTrigger(context); } +/** + * Checks if a security review has already been performed on this PR. + * Used to implement "run once" behavior for automatic security reviews. + */ +async function hasExistingSecurityReview( + octokit: Octokits, + context: ParsedGitHubContext, +): Promise { + const { owner, repo } = context.repository; + + try { + const comments = await octokit.rest.issues.listComments({ + owner, + repo, + issue_number: context.entityNumber, + per_page: 100, + }); + + const hasSecurityReview = comments.data.some((comment) => { + const isOurBot = + comment.user?.id === DROID_APP_BOT_ID || + (comment.user?.type === "Bot" && + comment.user?.login.toLowerCase().includes("droid")); + return isOurBot && comment.body?.includes(SECURITY_REVIEW_MARKER); + }); + + return hasSecurityReview; + } catch (error) { + console.warn("Failed to check for existing security review:", error); + return false; + } +} + type PrepareTagOptions = { context: GitHubContext; octokit: Octokits; @@ -36,16 +78,67 @@ export async function prepareTagExecution({ await checkHumanActor(octokit.rest, context); - const commentData = await createInitialComment(octokit.rest, context); - const commentId = commentData.id; - if (context.inputs.automaticReview && !context.isPR) { throw new Error("automatic_review requires a pull request context"); } + if (context.inputs.automaticSecurityReview && !context.isPR) { + throw new Error( + "automatic_security_review requires a pull request context", + ); + } + const commandContext = extractCommandFromContext(context); + // Determine if this is a security-related command for the initial comment + const isSecurityCommand = + context.inputs.automaticSecurityReview || + commandContext?.command === "security" || + commandContext?.command === "security-full"; + + const commentData = await createInitialComment( + octokit.rest, + context, + isSecurityCommand ? "security" : "default", + ); + const commentId = commentData.id; + + // Handle parallel review mode when both flags are set + if ( + context.inputs.automaticReview && + context.inputs.automaticSecurityReview + ) { + // Output flags for parallel workflow jobs + const runCodeReview = true; + let runSecurityReview = true; + + // Check if security review already exists on this PR (run once behavior) + const hasExisting = await hasExistingSecurityReview(octokit, context); + if (hasExisting) { + console.log( + "Security review already exists on this PR, skipping security", + ); + runSecurityReview = false; + } + + // Set outputs for downstream jobs + core.setOutput("run_code_review", runCodeReview.toString()); + core.setOutput("run_security_review", runSecurityReview.toString()); + + // For parallel mode, return early - individual jobs will run their own reviews + return { + skipped: false, + branchInfo: { + baseBranch: "", + currentBranch: "", + }, + mcpTools: "", + }; + } + if (context.inputs.automaticReview) { + core.setOutput("run_code_review", "true"); + core.setOutput("run_security_review", "false"); return prepareReviewMode({ context, octokit, @@ -54,6 +147,32 @@ export async function prepareTagExecution({ }); } + if (context.inputs.automaticSecurityReview) { + // Check if security review already exists on this PR (run once behavior) + const hasExisting = await hasExistingSecurityReview(octokit, context); + if (hasExisting) { + console.log("Security review already exists on this PR, skipping"); + return { + skipped: true, + reason: "security_review_exists", + branchInfo: { + baseBranch: "", + currentBranch: "", + }, + mcpTools: "", + }; + } + + core.setOutput("run_code_review", "false"); + core.setOutput("run_security_review", "true"); + return prepareSecurityReviewMode({ + context, + octokit, + githubToken, + trackingCommentId: commentId, + }); + } + if (commandContext?.command === "fill") { return prepareFillMode({ context, @@ -63,17 +182,58 @@ export async function prepareTagExecution({ }); } + // Handle explicit commands - set output flags for parallel workflow jobs + if (commandContext?.command === "review-security") { + core.setOutput("run_code_review", "true"); + core.setOutput("run_security_review", "true"); + return { + skipped: false, + branchInfo: { + baseBranch: "", + currentBranch: "", + }, + mcpTools: "", + }; + } + + if (commandContext?.command === "security") { + core.setOutput("run_code_review", "false"); + core.setOutput("run_security_review", "true"); + return { + skipped: false, + branchInfo: { + baseBranch: "", + currentBranch: "", + }, + mcpTools: "", + }; + } + + if (commandContext?.command === "security-full") { + return prepareSecurityScanMode({ + context, + octokit, + githubToken, + scanScope: { type: "full" }, + }); + } + + // @droid review or @droid (default) = code review only if ( commandContext?.command === "review" || !commandContext || commandContext.command === "default" ) { - return prepareReviewMode({ - context, - octokit, - githubToken, - trackingCommentId: commentId, - }); + core.setOutput("run_code_review", "true"); + core.setOutput("run_security_review", "false"); + return { + skipped: false, + branchInfo: { + baseBranch: "", + currentBranch: "", + }, + mcpTools: "", + }; } throw new Error(`Unexpected command: ${commandContext?.command}`); diff --git a/src/utils/parse-tools.ts b/src/utils/parse-tools.ts index 9f6e065..9020401 100644 --- a/src/utils/parse-tools.ts +++ b/src/utils/parse-tools.ts @@ -22,7 +22,10 @@ export function parseAllowedTools(args: string): string[] { return []; } - return value.split(",").map((tool) => tool.trim()).filter(Boolean); + return value + .split(",") + .map((tool) => tool.trim()) + .filter(Boolean); } export function normalizeDroidArgs(args: string): string { @@ -30,14 +33,16 @@ export function normalizeDroidArgs(args: string): string { return ""; } - return args - .replace(/--allowedTools/g, "--enabled-tools") - .replace(/--allowed-tools/g, "--enabled-tools") - .replace(/--enabledTools/g, "--enabled-tools") - .replace(/--disallowedTools/g, "--disabled-tools") - .replace(/--disabled-tools/g, "--disabled-tools") - .replace(/--disallowed-tools/g, "--disabled-tools") - // Strip unsupported MCP inline config flags to avoid CLI errors - .replace(/--mcp-config\s+(?:"[^"]*"|'[^']*'|[^\s]+)/g, "") - .trim(); + return ( + args + .replace(/--allowedTools/g, "--enabled-tools") + .replace(/--allowed-tools/g, "--enabled-tools") + .replace(/--enabledTools/g, "--enabled-tools") + .replace(/--disallowedTools/g, "--disabled-tools") + .replace(/--disabled-tools/g, "--disabled-tools") + .replace(/--disallowed-tools/g, "--disabled-tools") + // Strip unsupported MCP inline config flags to avoid CLI errors + .replace(/--mcp-config\s+(?:"[^"]*"|'[^']*'|[^\s]+)/g, "") + .trim() + ); } diff --git a/test/comment-logic.test.ts b/test/comment-logic.test.ts index 33bb2e6..b068949 100644 --- a/test/comment-logic.test.ts +++ b/test/comment-logic.test.ts @@ -279,9 +279,7 @@ describe("updateCommentBody", () => { }; const result = updateCommentBody(input); - expect(result).toContain( - "**Droid finished @testuser's task in 1m 15s**", - ); + expect(result).toContain("**Droid finished @testuser's task in 1m 15s**"); }); it("includes duration in error header", () => { diff --git a/test/create-prompt.test.ts b/test/create-prompt.test.ts index 4d9543b..f4176ff 100644 --- a/test/create-prompt.test.ts +++ b/test/create-prompt.test.ts @@ -26,6 +26,14 @@ describe("prepareContext", () => { allowedNonWriteUsers: "", trackProgress: false, automaticReview: false, + automaticSecurityReview: false, + securityModel: "", + securitySeverityThreshold: "medium", + securityBlockOnCritical: true, + securityBlockOnHigh: false, + securityNotifyTeam: "", + securityScanSchedule: false, + securityScanDays: 7, }, } as const; diff --git a/test/create-prompt/templates/fill-prompt.test.ts b/test/create-prompt/templates/fill-prompt.test.ts index 5d42bf4..baff84d 100644 --- a/test/create-prompt/templates/fill-prompt.test.ts +++ b/test/create-prompt/templates/fill-prompt.test.ts @@ -2,7 +2,9 @@ import { describe, expect, it } from "bun:test"; import { generateFillPrompt } from "../../../src/create-prompt/templates/fill-prompt"; import type { PreparedContext } from "../../../src/create-prompt/types"; -function createBaseContext(overrides: Partial = {}): PreparedContext { +function createBaseContext( + overrides: Partial = {}, +): PreparedContext { return { repository: "test-owner/test-repo", droidCommentId: "123", @@ -26,7 +28,9 @@ describe("generateFillPrompt", () => { const prompt = generateFillPrompt(context); expect(prompt).toContain("Procedure:"); - expect(prompt).toContain("gh pr view 42 --repo test-owner/test-repo --json title,body"); + expect(prompt).toContain( + "gh pr view 42 --repo test-owner/test-repo --json title,body", + ); expect(prompt).toContain("gh pr diff 42 --repo test-owner/test-repo"); expect(prompt).toContain("github_pr___update_pr_description"); expect(prompt).toContain("Do not proceed if required commands fail"); diff --git a/test/create-prompt/templates/review-prompt.test.ts b/test/create-prompt/templates/review-prompt.test.ts index 3833ded..4b086bd 100644 --- a/test/create-prompt/templates/review-prompt.test.ts +++ b/test/create-prompt/templates/review-prompt.test.ts @@ -28,42 +28,44 @@ describe("generateReviewPrompt", () => { const prompt = generateReviewPrompt(context); expect(prompt).toContain("Objectives:"); - expect(prompt).toContain("Re-check existing review comments"); + expect(prompt).toContain("Review the PR diff"); expect(prompt).toContain("git merge-base"); expect(prompt).toContain("git diff"); + expect(prompt).toContain("gh pr diff 42 --repo test-owner/test-repo"); expect(prompt).toContain( - "gh pr diff 42 --repo test-owner/test-repo", + "gh api repos/test-owner/test-repo/pulls/42/files", ); - expect(prompt).toContain("gh api repos/test-owner/test-repo/pulls/42/files"); - expect(prompt).toContain("github_inline_comment___create_inline_comment"); - expect(prompt).toContain("github_pr___resolve_review_thread"); - expect(prompt).toContain("every substantive comment must be inline on the changed line"); + expect(prompt).toContain("code-review-results.json"); + expect(prompt).toContain("Do NOT post inline comments"); }); it("emphasizes accuracy gates and bug detection guidelines", () => { const prompt = generateReviewPrompt(createBaseContext()); expect(prompt).toContain("How Many Findings to Return:"); - expect(prompt).toContain("Output all findings that the original author would fix"); + expect(prompt).toContain( + "Output all findings that the original author would fix", + ); expect(prompt).toContain("Key Guidelines for Bug Detection:"); expect(prompt).toContain("Priority Levels:"); expect(prompt).toContain("[P0]"); expect(prompt).toContain("Never raise purely stylistic"); - expect(prompt).toContain("Never repeat or re-raise an issue previously highlighted"); + expect(prompt).toContain( + "Never repeat or re-raise an issue previously highlighted", + ); }); - it("describes submission guidance", () => { + it("describes output format with Greptile-style summary", () => { const prompt = generateReviewPrompt(createBaseContext()); - expect(prompt).toContain("Prefer github_inline_comment___create_inline_comment"); - expect(prompt).toContain("gh api repos/test-owner/test-repo/pulls/42/reviews"); - expect(prompt).toContain("Do not approve or request changes"); - expect(prompt).toContain("github_pr___submit_review"); - expect(prompt).toContain("github_pr___resolve_review_thread"); - expect(prompt).toContain("skip submitting another comment to avoid redundancy"); - expect(prompt).toContain("Do not submit inline comments"); + expect(prompt).toContain("code-review-results.json"); + expect(prompt).toContain("github_comment___update_droid_comment"); expect(prompt).toContain( - "Do not escalate style/formatting into P0/P1 just to justify leaving an inline comment", + "Inline comments will be posted after all reviews complete", ); + expect(prompt).toContain("### Summary"); + expect(prompt).toContain("### Key Changes"); + expect(prompt).toContain("### Important Files Changed"); + expect(prompt).toContain("### Review Findings"); }); }); diff --git a/test/create-prompt/templates/security-report-prompt.test.ts b/test/create-prompt/templates/security-report-prompt.test.ts new file mode 100644 index 0000000..f2698d3 --- /dev/null +++ b/test/create-prompt/templates/security-report-prompt.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, test } from "bun:test"; +import { generateSecurityReportPrompt } from "../../../src/create-prompt/templates/security-report-prompt"; +import type { PreparedContext } from "../../../src/create-prompt/types"; + +describe("generateSecurityReportPrompt", () => { + const baseContext: PreparedContext = { + repository: "test-owner/test-repo", + triggerPhrase: "@droid", + eventData: { + eventName: "issue_comment", + commentId: "123", + issueNumber: "1", + isPR: false, + baseBranch: "main", + droidBranch: "droid/security-report-2024-01-15", + commentBody: "@droid security --full", + }, + githubContext: { + runId: "1234567890", + eventName: "issue_comment", + eventAction: "created", + repository: { + owner: "test-owner", + repo: "test-repo", + full_name: "test-owner/test-repo", + }, + actor: "test-user", + payload: {} as any, + entityNumber: 1, + isPR: false, + inputs: { + triggerPhrase: "@droid", + assigneeTrigger: "", + labelTrigger: "", + useStickyComment: false, + allowedBots: "", + allowedNonWriteUsers: "", + trackProgress: false, + automaticReview: false, + automaticSecurityReview: false, + securityModel: "", + securitySeverityThreshold: "high", + securityBlockOnCritical: true, + securityBlockOnHigh: false, + securityNotifyTeam: "@org/security-team", + securityScanSchedule: false, + securityScanDays: 7, + }, + }, + }; + + test("includes scan configuration for full repository scan", () => { + const prompt = generateSecurityReportPrompt( + baseContext, + { type: "full" }, + "droid/security-report-2024-01-15", + ); + + expect(prompt).toContain("full repository"); + expect(prompt).toContain("Entire repository"); + expect(prompt).toContain("droid/security-report-2024-01-15"); + expect(prompt).toContain(".factory/security/reports/security-report-"); + }); + + test("includes scan configuration for scheduled scan", () => { + const prompt = generateSecurityReportPrompt( + baseContext, + { type: "scheduled", days: 7 }, + "droid/security-report-2024-01-15", + ); + + expect(prompt).toContain("weekly scheduled"); + expect(prompt).toContain("Last 7 days of commits"); + expect(prompt).toContain("git log --since="); + }); + + test("includes security skills workflow", () => { + const prompt = generateSecurityReportPrompt( + baseContext, + { type: "full" }, + "droid/security-report-2024-01-15", + ); + + expect(prompt).toContain("threat-model-generation"); + expect(prompt).toContain("commit-security-scan"); + expect(prompt).toContain("vulnerability-validation"); + expect(prompt).toContain("security-review"); + }); + + test("includes PR creation instructions", () => { + const prompt = generateSecurityReportPrompt( + baseContext, + { type: "full" }, + "droid/security-report-2024-01-15", + ); + + expect(prompt).toContain("git checkout -b"); + expect(prompt).toContain("git push origin"); + expect(prompt).toContain("gh pr create"); + expect(prompt).toContain("fix(security):"); + }); + + test("includes report format template", () => { + const prompt = generateSecurityReportPrompt( + baseContext, + { type: "full" }, + "droid/security-report-2024-01-15", + ); + + expect(prompt).toContain("# Security Scan Report"); + expect(prompt).toContain("Executive Summary"); + expect(prompt).toContain( + "| Severity | Count | Auto-fixed | Manual Required |", + ); + expect(prompt).toContain("VULN-001"); + expect(prompt).toContain("STRIDE"); + expect(prompt).toContain("CWE"); + }); + + test("includes severity definitions", () => { + const prompt = generateSecurityReportPrompt( + baseContext, + { type: "full" }, + "droid/security-report-2024-01-15", + ); + + expect(prompt).toContain("CRITICAL"); + expect(prompt).toContain("HIGH"); + expect(prompt).toContain("MEDIUM"); + expect(prompt).toContain("LOW"); + expect(prompt).toContain("Immediately exploitable"); + }); + + test("includes security configuration from context", () => { + const prompt = generateSecurityReportPrompt( + baseContext, + { type: "full" }, + "droid/security-report-2024-01-15", + ); + + expect(prompt).toContain("Severity Threshold: high"); + expect(prompt).toContain("@org/security-team"); + }); + + test("includes threat model check instructions", () => { + const prompt = generateSecurityReportPrompt( + baseContext, + { type: "full" }, + "droid/security-report-2024-01-15", + ); + + expect(prompt).toContain(".factory/threat-model.md"); + expect(prompt).toContain("Threat Model Check"); + expect(prompt).toContain("90 days old"); + }); +}); diff --git a/test/create-prompt/templates/security-review-prompt.test.ts b/test/create-prompt/templates/security-review-prompt.test.ts new file mode 100644 index 0000000..5014fcd --- /dev/null +++ b/test/create-prompt/templates/security-review-prompt.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from "bun:test"; +import { generateSecurityReviewPrompt } from "../../../src/create-prompt/templates/security-review-prompt"; +import type { PreparedContext } from "../../../src/create-prompt/types"; + +function createBaseContext( + overrides: Partial = {}, +): PreparedContext { + return { + repository: "test-owner/test-repo", + droidCommentId: "123", + triggerPhrase: "@droid", + eventData: { + eventName: "issue_comment", + commentId: "456", + prNumber: "42", + isPR: true, + commentBody: "@droid security-review", + }, + githubContext: undefined, + ...overrides, + } as PreparedContext; +} + +describe("generateSecurityReviewPrompt", () => { + it("includes security context and skill workflow", () => { + const prompt = generateSecurityReviewPrompt(createBaseContext()); + + expect(prompt).toContain("security-focused code review"); + expect(prompt).toContain("## Security Skills Available"); + expect(prompt).toContain("threat-model-generation"); + expect(prompt).toContain("commit-security-scan"); + expect(prompt).toContain("vulnerability-validation"); + expect(prompt).toContain("security-review"); + expect(prompt).toContain("## Review Workflow"); + expect(prompt).toContain("gh pr diff 42 --repo test-owner/test-repo"); + expect(prompt).toContain( + "gh api repos/test-owner/test-repo/pulls/42/files", + ); + expect(prompt).toContain("security-review-results.json"); + expect(prompt).toContain("Do NOT post inline comments"); + }); + + it("lists STRIDE security categories", () => { + const prompt = generateSecurityReviewPrompt(createBaseContext()); + + expect(prompt).toContain("Spoofing"); + expect(prompt).toContain("Tampering"); + expect(prompt).toContain("Repudiation"); + expect(prompt).toContain("Information Disclosure"); + expect(prompt).toContain("Denial of Service"); + expect(prompt).toContain("Elevation of Privilege"); + }); + + it("includes severity definitions", () => { + const prompt = generateSecurityReviewPrompt(createBaseContext()); + + expect(prompt).toContain("CRITICAL"); + expect(prompt).toContain("HIGH"); + expect(prompt).toContain("MEDIUM"); + expect(prompt).toContain("LOW"); + }); + + it("includes security configuration from context", () => { + const contextWithConfig = createBaseContext({ + githubContext: { + inputs: { + securitySeverityThreshold: "high", + securityBlockOnCritical: true, + securityBlockOnHigh: true, + securityNotifyTeam: "@org/security-team", + }, + } as any, + }); + + const prompt = generateSecurityReviewPrompt(contextWithConfig); + + expect(prompt).toContain("Severity Threshold: high"); + expect(prompt).toContain("Block on Critical: true"); + expect(prompt).toContain("Block on High: true"); + expect(prompt).toContain("@org/security-team"); + }); +}); diff --git a/test/github/token.test.ts b/test/github/token.test.ts index d5983a6..a4d8a5a 100644 --- a/test/github/token.test.ts +++ b/test/github/token.test.ts @@ -34,15 +34,14 @@ describe("setupGitHubToken", () => { process.env.OVERRIDE_GITHUB_TOKEN = "override-token"; const setOutputSpy = spyOn(core, "setOutput").mockImplementation(() => {}); - const getIdTokenSpy = spyOn(core, "getIDToken").mockResolvedValue("oidc-token"); + const getIdTokenSpy = spyOn(core, "getIDToken").mockResolvedValue( + "oidc-token", + ); const result = await setupGitHubToken(); expect(result).toBe("override-token"); - expect(setOutputSpy).toHaveBeenCalledWith( - "GITHUB_TOKEN", - "override-token", - ); + expect(setOutputSpy).toHaveBeenCalledWith("GITHUB_TOKEN", "override-token"); expect(getIdTokenSpy).not.toHaveBeenCalled(); setOutputSpy.mockRestore(); @@ -59,7 +58,9 @@ describe("setupGitHubToken", () => { global.fetch = fetchMock as unknown as typeof fetch; const setOutputSpy = spyOn(core, "setOutput").mockImplementation(() => {}); - const getIdTokenSpy = spyOn(core, "getIDToken").mockResolvedValue("oidc-token"); + const getIdTokenSpy = spyOn(core, "getIDToken").mockResolvedValue( + "oidc-token", + ); const retrySpy = spyOn(retryModule, "retryWithBackoff").mockImplementation( (operation: () => Promise) => operation(), ); diff --git a/test/integration/fill-flow.test.ts b/test/integration/fill-flow.test.ts index 21ea50c..ed41bc5 100644 --- a/test/integration/fill-flow.test.ts +++ b/test/integration/fill-flow.test.ts @@ -1,11 +1,4 @@ -import { - afterEach, - beforeEach, - describe, - expect, - it, - spyOn, -} from "bun:test"; +import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test"; import path from "node:path"; import os from "node:os"; import { mkdtemp, readFile, rm } from "node:fs/promises"; @@ -93,17 +86,18 @@ describe("fill command integration", () => { } as any, }); - const octokit = { - rest: {}, - graphql: () => Promise.resolve({ - repository: { - pullRequest: { - baseRefName: "main", - headRefName: "feature/fill", - headRefOid: "abc123", - } - } - }) + const octokit = { + rest: {}, + graphql: () => + Promise.resolve({ + repository: { + pullRequest: { + baseRefName: "main", + headRefName: "feature/fill", + headRefOid: "abc123", + }, + }, + }), } as any; graphqlSpy = spyOn(octokit, "graphql").mockResolvedValue({ @@ -112,8 +106,8 @@ describe("fill command integration", () => { baseRefName: "main", headRefName: "feature/fill", headRefOid: "abc123", - } - } + }, + }, }); const result = await prepareTagExecution({ diff --git a/test/integration/review-flow.test.ts b/test/integration/review-flow.test.ts index 61bbf6e..3fdc67a 100644 --- a/test/integration/review-flow.test.ts +++ b/test/integration/review-flow.test.ts @@ -1,14 +1,7 @@ -import { - afterEach, - beforeEach, - describe, - expect, - it, - spyOn, -} from "bun:test"; +import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test"; import path from "node:path"; import os from "node:os"; -import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { mkdtemp, rm } from "node:fs/promises"; import { prepareTagExecution } from "../../src/tag"; import { createMockContext } from "../mockContext"; import * as createInitial from "../../src/github/operations/comments/create-initial"; @@ -32,8 +25,6 @@ describe("review command integration", () => { process.env.RUNNER_TEMP = tmpDir; process.env.DROID_ARGS = ""; - - createCommentSpy = spyOn( createInitial, "createInitialComment", @@ -95,76 +86,141 @@ describe("review command integration", () => { } as any, }); - const octokit = { - rest: {}, - graphql: () => Promise.resolve({ - repository: { - pullRequest: { - baseRefName: "main", - headRefName: "feature/review", - headRefOid: "def456", - } - } - }) + const octokit = { + rest: {}, + graphql: () => + Promise.resolve({ + repository: { + pullRequest: { + baseRefName: "main", + headRefName: "feature/review", + headRefOid: "def456", + }, + }, + }), } as any; graphqlSpy = spyOn(octokit, "graphql").mockResolvedValue({ repository: { pullRequest: { baseRefName: "main", - headRefName: "feature/review", + headRefName: "feature/review", headRefOid: "def456", - } - } + }, + }, + }); + + const result = await prepareTagExecution({ + context, + octokit, + githubToken: "token", + }); + + // In the parallel workflow, @droid review sets output flags and returns early + // The actual review is done by downstream workflow jobs + expect(result.skipped).toBe(false); + + // Verify output flags were set correctly for code review only + const runCodeReviewCall = setOutputSpy.mock.calls.find( + (call: unknown[]) => call[0] === "run_code_review", + ) as [string, string] | undefined; + const runSecurityReviewCall = setOutputSpy.mock.calls.find( + (call: unknown[]) => call[0] === "run_security_review", + ) as [string, string] | undefined; + + expect(runCodeReviewCall?.[1]).toBe("true"); + expect(runSecurityReviewCall?.[1]).toBe("false"); + }); + + it("sets both review flags for @droid review security", async () => { + const context = createMockContext({ + eventName: "issue_comment", + isPR: true, + actor: "human-reviewer", + entityNumber: 7, + repository: { + owner: "test-owner", + repo: "test-repo", + full_name: "test-owner/test-repo", + }, + payload: { + comment: { + id: 888, + body: "@droid review security", + user: { login: "human-reviewer" }, + created_at: "2024-02-02T00:00:00Z", + }, + issue: { + number: 7, + pull_request: {}, + }, + } as any, }); + const octokit = { rest: {} } as any; + const result = await prepareTagExecution({ context, octokit, githubToken: "token", }); - expect(result.commentId).toBe(202); - expect(graphqlSpy).toHaveBeenCalled(); - expect(mcpSpy).toHaveBeenCalledWith( - expect.objectContaining({ - allowedTools: expect.arrayContaining([ - "github_pr___list_review_comments", - "github_pr___submit_review", - "github_inline_comment___create_inline_comment", - "github_pr___resolve_review_thread", - ]), - }), - ); - - const promptPath = path.join( - process.env.RUNNER_TEMP!, - "droid-prompts", - "droid-prompt.txt", - ); - const prompt = await readFile(promptPath, "utf8"); - - expect(prompt).toContain("You are performing an automated code review"); - expect(prompt).toContain("github_inline_comment___create_inline_comment"); - expect(prompt).toContain("How Many Findings to Return:"); - expect(prompt).toContain("Output all findings that the original author would fix"); - expect(prompt).toContain("Key Guidelines for Bug Detection:"); - expect(prompt).toContain("Priority Levels:"); - expect(prompt).toContain("gh pr view 7 --repo test-owner/test-repo --json comments,reviews"); - expect(prompt).toContain("every substantive comment must be inline on the changed line"); - expect(prompt).toContain("github_pr___resolve_review_thread"); - - const droidArgsCall = setOutputSpy.mock.calls.find( - (call: unknown[]) => call[0] === "droid_args", + expect(result.skipped).toBe(false); + + const runCodeReviewCall = setOutputSpy.mock.calls.find( + (call: unknown[]) => call[0] === "run_code_review", + ) as [string, string] | undefined; + const runSecurityReviewCall = setOutputSpy.mock.calls.find( + (call: unknown[]) => call[0] === "run_security_review", + ) as [string, string] | undefined; + + expect(runCodeReviewCall?.[1]).toBe("true"); + expect(runSecurityReviewCall?.[1]).toBe("true"); + }); + + it("sets security flag only for @droid security", async () => { + const context = createMockContext({ + eventName: "issue_comment", + isPR: true, + actor: "human-reviewer", + entityNumber: 7, + repository: { + owner: "test-owner", + repo: "test-repo", + full_name: "test-owner/test-repo", + }, + payload: { + comment: { + id: 888, + body: "@droid security", + user: { login: "human-reviewer" }, + created_at: "2024-02-02T00:00:00Z", + }, + issue: { + number: 7, + pull_request: {}, + }, + } as any, + }); + + const octokit = { rest: {} } as any; + + const result = await prepareTagExecution({ + context, + octokit, + githubToken: "token", + }); + + expect(result.skipped).toBe(false); + + const runCodeReviewCall = setOutputSpy.mock.calls.find( + (call: unknown[]) => call[0] === "run_code_review", + ) as [string, string] | undefined; + const runSecurityReviewCall = setOutputSpy.mock.calls.find( + (call: unknown[]) => call[0] === "run_security_review", ) as [string, string] | undefined; - expect(droidArgsCall?.[1]).toContain( - "github_pr___list_review_comments", - ); - expect(droidArgsCall?.[1]).toContain("github_pr___submit_review"); - expect(droidArgsCall?.[1]).toContain( - "github_inline_comment___create_inline_comment", - ); - expect(droidArgsCall?.[1]).toContain("github_pr___resolve_review_thread"); + expect(runCodeReviewCall?.[1]).toBe("false"); + expect(runSecurityReviewCall?.[1]).toBe("true"); }); }); diff --git a/test/mockContext.ts b/test/mockContext.ts index 657b118..4c197a7 100644 --- a/test/mockContext.ts +++ b/test/mockContext.ts @@ -19,6 +19,14 @@ const defaultInputs = { allowedNonWriteUsers: "", trackProgress: false, automaticReview: false, + automaticSecurityReview: false, + securityModel: "", + securitySeverityThreshold: "medium", + securityBlockOnCritical: true, + securityBlockOnHigh: false, + securityNotifyTeam: "", + securityScanSchedule: false, + securityScanDays: 7, }; const defaultRepository = { diff --git a/test/modes/tag.test.ts b/test/modes/tag.test.ts index 1cd012f..6a4ff72 100644 --- a/test/modes/tag.test.ts +++ b/test/modes/tag.test.ts @@ -52,4 +52,17 @@ describe("shouldTriggerTag", () => { expect(shouldTriggerTag(contextWithAutomaticReview)).toBe(true); }); + + test("returns true for PR contexts when automaticSecurityReview is enabled", () => { + const contextWithAutomaticSecurityReview = createMockContext({ + eventName: "issue_comment", + isPR: true, + inputs: { + ...createMockContext().inputs, + automaticSecurityReview: true, + }, + }); + + expect(shouldTriggerTag(contextWithAutomaticSecurityReview)).toBe(true); + }); }); diff --git a/test/modes/tag/fill-command.test.ts b/test/modes/tag/fill-command.test.ts index 46b6cdf..ad3f061 100644 --- a/test/modes/tag/fill-command.test.ts +++ b/test/modes/tag/fill-command.test.ts @@ -1,11 +1,4 @@ -import { - afterEach, - beforeEach, - describe, - expect, - it, - spyOn, -} from "bun:test"; +import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test"; import * as core from "@actions/core"; import { prepareFillMode } from "../../../src/tag/commands/fill"; import { createMockContext } from "../../mockContext"; diff --git a/test/modes/tag/review-command.test.ts b/test/modes/tag/review-command.test.ts index e31db2c..d09be4a 100644 --- a/test/modes/tag/review-command.test.ts +++ b/test/modes/tag/review-command.test.ts @@ -260,7 +260,9 @@ describe("prepareReviewMode", () => { const droidArgsCall = setOutputSpy.mock.calls.find( (call: unknown[]) => call[0] === "droid_args", ) as [string, string] | undefined; - expect(droidArgsCall?.[1]).toContain('--model "claude-sonnet-4-5-20250929"'); + expect(droidArgsCall?.[1]).toContain( + '--model "claude-sonnet-4-5-20250929"', + ); }); it("does not add --model flag when REVIEW_MODEL is empty", async () => { diff --git a/test/modes/tag/security-review-command.test.ts b/test/modes/tag/security-review-command.test.ts new file mode 100644 index 0000000..66a2b88 --- /dev/null +++ b/test/modes/tag/security-review-command.test.ts @@ -0,0 +1,335 @@ +import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test"; +import * as core from "@actions/core"; +import { prepareSecurityReviewMode } from "../../../src/tag/commands/security-review"; +import { createMockContext } from "../../mockContext"; + +import * as prFetcher from "../../../src/github/data/pr-fetcher"; +import * as promptModule from "../../../src/create-prompt"; +import * as mcpInstaller from "../../../src/mcp/install-mcp-server"; +import * as comments from "../../../src/github/operations/comments/create-initial"; + +const MOCK_PR_DATA = { + baseRefName: "main", + headRefName: "feature/security-review", + headRefOid: "123abc", +} as const; + +describe("prepareSecurityReviewMode", () => { + const originalArgs = process.env.DROID_ARGS; + const originalReviewModel = process.env.REVIEW_MODEL; + const originalSecurityModel = process.env.SECURITY_MODEL; + let fetchPRSpy: ReturnType; + let promptSpy: ReturnType; + let mcpSpy: ReturnType; + let setOutputSpy: ReturnType; + let createInitialSpy: ReturnType; + let exportVariableSpy: ReturnType; + + beforeEach(() => { + process.env.DROID_ARGS = ""; + delete process.env.REVIEW_MODEL; + delete process.env.SECURITY_MODEL; + + fetchPRSpy = spyOn(prFetcher, "fetchPRBranchData").mockResolvedValue({ + baseRefName: MOCK_PR_DATA.baseRefName, + headRefName: MOCK_PR_DATA.headRefName, + headRefOid: MOCK_PR_DATA.headRefOid, + }); + + promptSpy = spyOn(promptModule, "createPrompt").mockResolvedValue(); + mcpSpy = spyOn(mcpInstaller, "prepareMcpTools").mockResolvedValue( + "mock-config", + ); + setOutputSpy = spyOn(core, "setOutput").mockImplementation(() => {}); + createInitialSpy = spyOn( + comments, + "createInitialComment", + ).mockResolvedValue({ id: 777 } as any); + exportVariableSpy = spyOn(core, "exportVariable").mockImplementation( + () => {}, + ); + }); + + afterEach(() => { + fetchPRSpy.mockRestore(); + promptSpy.mockRestore(); + mcpSpy.mockRestore(); + setOutputSpy.mockRestore(); + createInitialSpy.mockRestore(); + exportVariableSpy.mockRestore(); + + process.env.DROID_ARGS = originalArgs; + if (originalReviewModel !== undefined) { + process.env.REVIEW_MODEL = originalReviewModel; + } else { + delete process.env.REVIEW_MODEL; + } + if (originalSecurityModel !== undefined) { + process.env.SECURITY_MODEL = originalSecurityModel; + } else { + delete process.env.SECURITY_MODEL; + } + }); + + it("prepares security review flow with limited toolset when tracking comment exists", async () => { + const context = createMockContext({ + eventName: "issue_comment", + isPR: true, + payload: { + comment: { + id: 101, + body: "@droid security-review", + }, + } as any, + entityNumber: 24, + }); + + const octokit = { rest: {}, graphql: () => {} } as any; + + const result = await prepareSecurityReviewMode({ + context, + octokit, + githubToken: "token", + trackingCommentId: 555, + }); + + expect(fetchPRSpy).toHaveBeenCalledWith({ + octokits: octokit, + repository: context.repository, + prNumber: 24, + }); + expect(promptSpy).toHaveBeenCalled(); + expect(mcpSpy).toHaveBeenCalledWith( + expect.objectContaining({ + allowedTools: expect.arrayContaining([ + "Execute", + "github_comment___update_droid_comment", + "github_inline_comment___create_inline_comment", + "github_pr___list_review_comments", + "github_pr___submit_review", + "github_pr___resolve_review_thread", + ]), + }), + ); + expect(createInitialSpy).not.toHaveBeenCalled(); + expect(result.commentId).toBe(555); + expect(result.branchInfo.baseBranch).toBe("main"); + expect(result.branchInfo.currentBranch).toBe("feature/security-review"); + expect(result.branchInfo.droidBranch).toBeUndefined(); + + const droidArgsCall = setOutputSpy.mock.calls.find( + (call: unknown[]) => call[0] === "droid_args", + ) as [string, string] | undefined; + expect(droidArgsCall?.[1]).toContain("Execute"); + [ + "github_comment___update_droid_comment", + "github_inline_comment___create_inline_comment", + "github_pr___list_review_comments", + "github_pr___submit_review", + "github_pr___delete_comment", + "github_pr___minimize_comment", + "github_pr___reply_to_comment", + "github_pr___resolve_review_thread", + ].forEach((tool) => { + expect(droidArgsCall?.[1]).toContain(tool); + }); + + expect(exportVariableSpy).toHaveBeenCalledWith( + "DROID_EXEC_RUN_TYPE", + "droid-security-review", + ); + }); + + it("creates tracking comment when not provided", async () => { + const context = createMockContext({ + eventName: "issue_comment", + isPR: true, + payload: { + comment: { + id: 102, + body: "@droid security-review", + }, + } as any, + entityNumber: 25, + }); + + const octokit = { rest: {}, graphql: () => {} } as any; + + const result = await prepareSecurityReviewMode({ + context, + octokit, + githubToken: "token", + }); + + expect(createInitialSpy).toHaveBeenCalled(); + expect(result.commentId).toBe(777); + }); + + it("throws when invoked on non-PR context", async () => { + const context = createMockContext({ isPR: false }); + + await expect( + prepareSecurityReviewMode({ + context, + octokit: { rest: {}, graphql: () => {} } as any, + githubToken: "token", + }), + ).rejects.toThrow( + "Security review command is only supported on pull requests", + ); + }); + + it("adds --model flag when REVIEW_MODEL is set", async () => { + process.env.REVIEW_MODEL = "claude-sonnet-4-5-20250929"; + + const context = createMockContext({ + eventName: "issue_comment", + isPR: true, + payload: { + comment: { + id: 103, + body: "@droid security-review", + }, + } as any, + entityNumber: 26, + }); + + const octokit = { rest: {}, graphql: () => {} } as any; + + await prepareSecurityReviewMode({ + context, + octokit, + githubToken: "token", + trackingCommentId: 556, + }); + + const droidArgsCall = setOutputSpy.mock.calls.find( + (call: unknown[]) => call[0] === "droid_args", + ) as [string, string] | undefined; + expect(droidArgsCall?.[1]).toContain( + '--model "claude-sonnet-4-5-20250929"', + ); + }); + + it("does not add --model flag when REVIEW_MODEL is empty", async () => { + process.env.REVIEW_MODEL = ""; + + const context = createMockContext({ + eventName: "issue_comment", + isPR: true, + payload: { + comment: { + id: 104, + body: "@droid security-review", + }, + } as any, + entityNumber: 27, + }); + + const octokit = { rest: {}, graphql: () => {} } as any; + + await prepareSecurityReviewMode({ + context, + octokit, + githubToken: "token", + trackingCommentId: 557, + }); + + const droidArgsCall = setOutputSpy.mock.calls.find( + (call: unknown[]) => call[0] === "droid_args", + ) as [string, string] | undefined; + expect(droidArgsCall?.[1]).not.toContain("--model"); + }); + + it("outputs install_security_skills flag", async () => { + const context = createMockContext({ + eventName: "issue_comment", + isPR: true, + payload: { + comment: { + id: 105, + body: "@droid security-review", + }, + } as any, + entityNumber: 28, + }); + + const octokit = { rest: {}, graphql: () => {} } as any; + + await prepareSecurityReviewMode({ + context, + octokit, + githubToken: "token", + trackingCommentId: 558, + }); + + const installSkillsCall = setOutputSpy.mock.calls.find( + (call: unknown[]) => call[0] === "install_security_skills", + ) as [string, string] | undefined; + expect(installSkillsCall?.[1]).toBe("true"); + }); + + it("prefers SECURITY_MODEL over REVIEW_MODEL", async () => { + process.env.SECURITY_MODEL = "gpt-5.1-codex"; + process.env.REVIEW_MODEL = "claude-sonnet-4-5-20250929"; + + const context = createMockContext({ + eventName: "issue_comment", + isPR: true, + payload: { + comment: { + id: 106, + body: "@droid security-review", + }, + } as any, + entityNumber: 29, + }); + + const octokit = { rest: {}, graphql: () => {} } as any; + + await prepareSecurityReviewMode({ + context, + octokit, + githubToken: "token", + trackingCommentId: 559, + }); + + const droidArgsCall = setOutputSpy.mock.calls.find( + (call: unknown[]) => call[0] === "droid_args", + ) as [string, string] | undefined; + expect(droidArgsCall?.[1]).toContain('--model "gpt-5.1-codex"'); + expect(droidArgsCall?.[1]).not.toContain("claude-sonnet"); + }); + + it("falls back to REVIEW_MODEL when SECURITY_MODEL is not set", async () => { + process.env.REVIEW_MODEL = "claude-sonnet-4-5-20250929"; + + const context = createMockContext({ + eventName: "issue_comment", + isPR: true, + payload: { + comment: { + id: 107, + body: "@droid security-review", + }, + } as any, + entityNumber: 30, + }); + + const octokit = { rest: {}, graphql: () => {} } as any; + + await prepareSecurityReviewMode({ + context, + octokit, + githubToken: "token", + trackingCommentId: 560, + }); + + const droidArgsCall = setOutputSpy.mock.calls.find( + (call: unknown[]) => call[0] === "droid_args", + ) as [string, string] | undefined; + expect(droidArgsCall?.[1]).toContain( + '--model "claude-sonnet-4-5-20250929"', + ); + }); +}); diff --git a/test/permissions.test.ts b/test/permissions.test.ts index c7c0b3c..4db439f 100644 --- a/test/permissions.test.ts +++ b/test/permissions.test.ts @@ -68,6 +68,14 @@ describe("checkWritePermissions", () => { allowedNonWriteUsers: "", trackProgress: false, automaticReview: false, + automaticSecurityReview: false, + securityModel: "", + securitySeverityThreshold: "medium", + securityBlockOnCritical: true, + securityBlockOnHigh: false, + securityNotifyTeam: "", + securityScanSchedule: false, + securityScanDays: 7, }, }); diff --git a/test/prepare-context.test.ts b/test/prepare-context.test.ts index e228822..d391956 100644 --- a/test/prepare-context.test.ts +++ b/test/prepare-context.test.ts @@ -72,7 +72,9 @@ describe("parseEnvVarsWithContext", () => { test("should throw error when DROID_BRANCH is missing", () => { expect(() => prepareContext(mockIssueCommentContext, "12345", "main"), - ).toThrow("issue_comment on issues requires droidBranch and baseBranch"); + ).toThrow( + "issue_comment on issues requires droidBranch and baseBranch", + ); }); test("should throw error when BASE_BRANCH is missing", () => { @@ -83,7 +85,9 @@ describe("parseEnvVarsWithContext", () => { undefined, "droid/issue-67890-20240101-1200", ), - ).toThrow("issue_comment on issues requires droidBranch and baseBranch"); + ).toThrow( + "issue_comment on issues requires droidBranch and baseBranch", + ); }); }); diff --git a/test/trigger-validation.test.ts b/test/trigger-validation.test.ts index 3ca63f0..46bdf8f 100644 --- a/test/trigger-validation.test.ts +++ b/test/trigger-validation.test.ts @@ -21,7 +21,6 @@ import type { import type { ParsedGitHubContext } from "../src/github/context"; describe("checkContainsTrigger", () => { - // TODO: Assignee and label triggers are disabled until instruct mode is implemented. // These tests verify the triggers are no-ops for now. describe("assignee trigger (disabled)", () => { diff --git a/test/utils/retry.test.ts b/test/utils/retry.test.ts index 36a58f0..84a584d 100644 --- a/test/utils/retry.test.ts +++ b/test/utils/retry.test.ts @@ -5,14 +5,14 @@ describe("retryWithBackoff", () => { let timeoutSpy: ReturnType; beforeEach(() => { - timeoutSpy = spyOn(globalThis, "setTimeout").mockImplementation( - ((handler: Parameters[0]) => { - if (typeof handler === "function") { - handler(); - } - return 0 as unknown as ReturnType; - }) as unknown as typeof setTimeout, - ); + timeoutSpy = spyOn(globalThis, "setTimeout").mockImplementation((( + handler: Parameters[0], + ) => { + if (typeof handler === "function") { + handler(); + } + return 0 as unknown as ReturnType; + }) as unknown as typeof setTimeout); }); afterEach(() => { @@ -64,7 +64,9 @@ describe("retryWithBackoff", () => { expect(attempts).toBe(2); expect(timeoutSpy).toHaveBeenCalledTimes(1); - const firstCall = timeoutSpy.mock.calls[0] as Parameters | undefined; + const firstCall = timeoutSpy.mock.calls[0] as + | Parameters + | undefined; expect(firstCall?.[1]).toBe(5); }); });