Add GitHub fallback for skill loading, rebase on dev

- load-skill.ts: try local plugin cache first, then fetch from GitHub
- Re-wire skill loading after rebase (review-prompt.ts removed by PR #59)
- Inject skill into candidates and validator prompt templates

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

Author: factory-nizar

README.md

@@ -182,6 +182,7 @@ Additional checks for this codebase:
 ```
 
 These guidelines are automatically loaded and injected into all review prompts (code review, security review, and validation passes). No workflow changes needed.
+
 ## Security Skills
 
 The security review uses specialized Factory skills installed from the public `Factory-AI/skills` repository:

src/create-prompt/index.ts

@@ -304,6 +304,7 @@ export type PromptCreationOptions = {
   includeActionsTools?: boolean;
   reviewArtifacts?: ReviewArtifacts;
   outputFilePath?: string;
+  reviewSkillContent?: string;
 };
 
 export async function createPrompt({
@@ -318,6 +319,7 @@ export async function createPrompt({
   includeActionsTools = false,
   reviewArtifacts,
   outputFilePath,
+  reviewSkillContent,
 }: PromptCreationOptions) {
   try {
     const droidCommentId = commentId?.toString();
@@ -334,6 +336,10 @@ export async function createPrompt({
       preparedContext.outputFilePath = outputFilePath;
     }
 
+    if (reviewSkillContent) {
+      preparedContext.reviewSkillContent = reviewSkillContent;
+    }
+
     await mkdir(`${process.env.RUNNER_TEMP || "/tmp"}/droid-prompts`, {
       recursive: true,
     });

src/create-prompt/templates/review-candidates-prompt.ts

@@ -1,3 +1,4 @@
+import { formatSkillSection } from "../../utils/load-skill";
 import type { PreparedContext } from "../types";
 
 export function generateReviewCandidatesPrompt(
@@ -30,7 +31,7 @@ export function generateReviewCandidatesPrompt(
   return `You are a senior staff software engineer and expert code reviewer.
 
 Your task: Review PR #${prNumber} in ${repoFullName} and generate a JSON file with **high-confidence, actionable** review comments that pinpoint genuine issues.
-
+${formatSkillSection(context.reviewSkillContent)}
 <context>
 Repo: ${repoFullName}
 PR Number: ${prNumber}
@@ -54,20 +55,7 @@ Precomputed data files:
 <review_guidelines>
 - You are currently checked out to the PR branch.
 - Review ALL modified files in the PR branch.
-- Focus on: functional correctness, syntax errors, logic bugs, broken dependencies/contracts/tests, security issues, and performance problems.
-- High-signal bug patterns to actively check for (only comment when evidenced in the diff):
-  - Null/undefined/Optional dereferences; missing-key errors on untrusted/external dict/JSON payloads
-  - Resource leaks (unclosed files/streams/connections; missing cleanup on error paths)
-  - Injection vulnerabilities (SQL injection, XSS, command/template injection) and auth/security invariant violations
-  - OAuth/CSRF invariants: state must be per-flow unpredictable and validated; avoid deterministic/predictable state or missing state checks
-  - Concurrency/race/atomicity hazards (TOCTOU, lost updates, unsafe shared state, process/thread lifecycle bugs)
-  - Missing error handling for critical operations (network, persistence, auth, migrations, external APIs)
-  - Wrong-variable/shadowing mistakes; contract mismatches (serializer/validated_data, interfaces/abstract methods)
-  - Type-assumption bugs (e.g., numeric ops on datetime/strings, ordering key type mismatches)
-  - Offset/cursor/pagination semantic mismatches (off-by-one, prev/next behavior, commit semantics)
 - Do NOT duplicate comments already in \`${commentsPath}\`.
-- Only flag issues you are confident about—avoid speculative or stylistic nitpicks.
-- **Confidence calibration:** For each finding, honestly assess how certain you are. Mark findings as P0 only if you are virtually certain of a crash/exploit. Mark as P1 for high-confidence correctness/security issues. Use P2 for findings where the bug is plausible but you cannot fully verify the trigger path from the available context. This severity rating will be used downstream for filtering.
 </review_guidelines>
 
 <triage_phase>
@@ -194,21 +182,9 @@ Write output to \`${reviewCandidatesPath}\` using this exact schema:
 - **comments**: Array of comment objects
   - \`path\`: Relative file path (e.g., "src/index.ts")
   - \`body\`: Comment text starting with priority tag [P0|P1|P2], then title, then 1 paragraph explanation
-    If you have **high confidence** a fix will address the issue and won’t break CI, append a GitHub suggestion block:
-
-    \`\`\`suggestion
-    <replacement code>
-    \`\`\`
-
-    **Suggestion rules:**
-    - Keep suggestion blocks ≤ 100 lines
-    - Preserve exact leading whitespace
-    - Use RIGHT-side anchors only; do not include removed/LEFT-side lines
-    - For insert-only suggestions, repeat the anchor line unchanged, then append new lines
   - \`line\`: Target line number (single-line) or end line number (multi-line). Must be ≥ 0.
   - \`startLine\`: \`null\` for single-line comments, or start line number for multi-line comments
-  - \`side\`: "RIGHT" for new/modified code (default). Use "LEFT" only for removed code **without** suggestions.
-    If you include a suggestion block, choose a RIGHT-side anchor and keep it unchanged so the validator can reuse it.
+  - \`side\`: "RIGHT" for new/modified code (default), "LEFT" only for removed code
   - \`commit_id\`: "${prHeadSha}"
 
 - **reviewSummary**:

src/create-prompt/templates/review-validator-prompt.ts

@@ -1,3 +1,4 @@
+import { formatSkillSection } from "../../utils/load-skill";
 import type { PreparedContext } from "../types";
 
 export function generateReviewValidatorPrompt(
@@ -33,7 +34,7 @@ export function generateReviewValidatorPrompt(
   return `You are validating candidate review comments for PR #${prNumber} in ${repoFullName}.
 
 IMPORTANT: This is Phase 2 (validator) of a two-pass review pipeline.
-
+${formatSkillSection(context.reviewSkillContent)}
 ### Context
 
 * Repo: ${repoFullName}
@@ -91,44 +92,11 @@ Read:
 
 ## Phase 2: Validate candidates
 
-Apply the same Reporting Gate as review:
-
-### Approve ONLY if at least one is true
-* Definite runtime failure
-* Incorrect logic with a concrete trigger path and wrong outcome
-* Security vulnerability with realistic exploit
-* Data corruption/loss
-* Breaking contract change (discoverable in code/tests)
+Apply the Reporting Gate, confidence calibration, and deduplication rules from the review methodology above.
 
-Reject if ANY of these are true:
-* It's speculative / "might" without a concrete trigger
-* It's stylistic / naming / formatting
+Additionally reject if:
 * It's not anchored to a valid changed line
-* It's already reported (dedupe against existing comments)
-* The anchor (path/side/line/startLine) would need to change to make the suggestion work — reject instead
-* It flags missing error handling / try-catch for a code path that won't crash in practice (e.g., the caller already handles the error, or the input is validated upstream)
-* It describes a hypothetical race condition or timing issue without identifying the specific concurrent access pattern that triggers it
-* It's about code that appears in the diff but is not part of the PR's primary change — e.g., adjacent functions, unrelated files in a multi-subsystem PR, or code from a different PR's changes that happen to be visible in context
-
-### Confidence-based filtering
-
-Pay attention to the candidate's priority level:
-- **P0 findings**: Approve if the trigger path checks out. These should be definite crashes/exploits.
-- **P1 findings**: Approve if you can verify the logic error or security issue is real.
-- **P2 findings**: Reject by default. Only approve a P2 finding if ALL of these are true: (1) you can independently verify the bug exists by examining the code, (2) the bug has a concrete trigger that a user or caller could realistically hit, and (3) the finding is NOT about edge cases, defensive coding, or style. When in doubt about a P2, reject it.
-
-### Deduplication (STRICT)
-
-Before approving a candidate, check for duplicates:
-1. **Among candidates**: If two or more candidates describe the same underlying bug (same root cause, even if anchored to different lines or worded differently), approve only the ONE with the best anchor and clearest explanation. Reject the rest with reason "duplicate of candidate N".
-2. **Against existing comments**: If a candidate repeats an issue already covered by an existing PR comment (from \`${commentsPath}\`), reject it with reason "already reported in existing comments".
-3. Same file + overlapping line range + same issue = duplicate, even if the body text differs.
-
-Suggestion block rules (minimal):
-* Preserve exact leading whitespace and keep blocks ≤ 100 lines
-* Use RIGHT-side anchors only; do not include removed/LEFT-side lines
-* For insert-only suggestions, repeat the anchor line unchanged, then append new lines
-* Do not change the anchor fields (path/side/line/startLine) from the candidate — only edit the body
+* It's already reported (dedupe against existing comments in \`${commentsPath}\`)
 
 When rejecting, write a concise reason.
 

src/create-prompt/types.ts

@@ -118,4 +118,5 @@ export type PreparedContext = CommonFields & {
   };
   reviewArtifacts?: ReviewArtifacts;
   outputFilePath?: string;
+  reviewSkillContent?: string;
 };

src/entrypoints/generate-review-prompt.ts

@@ -15,6 +15,7 @@ import { prepareMcpTools } from "../mcp/install-mcp-server";
 import { generateReviewCandidatesPrompt } from "../create-prompt/templates/review-candidates-prompt";
 import { generateSecurityReviewPrompt } from "../create-prompt/templates/security-review-prompt";
 import { normalizeDroidArgs, parseAllowedTools } from "../utils/parse-tools";
+import { loadSkill } from "../utils/load-skill";
 
 async function run() {
   try {
@@ -97,6 +98,9 @@ async function run() {
     // to write structured findings for the combine step
     const outputFilePath = process.env.DROID_OUTPUT_FILE || undefined;
 
+    const reviewSkillContent =
+      reviewType === "code" ? await loadSkill("review") : undefined;
+
     await createPrompt({
       githubContext: context,
       commentId,
@@ -108,6 +112,7 @@ async function run() {
       generatePrompt,
       reviewArtifacts,
       outputFilePath,
+      reviewSkillContent,
     });
 
     // Set run type

src/mcp/github-pr-server.ts

@@ -624,7 +624,12 @@ export function createGitHubPRServer({
             path: z
               .string()
               .describe("The file path to comment on (e.g., 'src/index.js')"),
-            body: z.string().min(1).describe("The comment text (supports markdown and GitHub code suggestion blocks)"),
+            body: z
+              .string()
+              .min(1)
+              .describe(
+                "The comment text (supports markdown and GitHub code suggestion blocks)",
+              ),
             line: z
               .number()
               .int()

src/tag/commands/review-validator.ts

@@ -9,6 +9,7 @@ import { prepareMcpTools } from "../../mcp/install-mcp-server";
 import { normalizeDroidArgs, parseAllowedTools } from "../../utils/parse-tools";
 import type { PrepareResult } from "../../prepare/types";
 import { generateReviewValidatorPrompt } from "../../create-prompt/templates/review-validator-prompt";
+import { loadSkill } from "../../utils/load-skill";
 
 export async function prepareReviewValidatorMode({
   context,
@@ -45,6 +46,8 @@ export async function prepareReviewValidatorMode({
     descriptionPath: `${promptsDir}/pr_description.txt`,
   };
 
+  const reviewSkillContent = await loadSkill("review");
+
   await createPrompt({
     githubContext: context,
     commentId: trackingCommentId,
@@ -56,6 +59,7 @@ export async function prepareReviewValidatorMode({
     },
     generatePrompt: generateReviewValidatorPrompt,
     reviewArtifacts,
+    reviewSkillContent,
   });
 
   core.exportVariable("DROID_EXEC_RUN_TYPE", "droid-review");

src/tag/commands/review.ts

@@ -9,6 +9,7 @@ import { createInitialComment } from "../../github/operations/comments/create-in
 import { normalizeDroidArgs, parseAllowedTools } from "../../utils/parse-tools";
 import { isEntityContext } from "../../github/context";
 import { generateReviewCandidatesPrompt } from "../../create-prompt/templates/review-candidates-prompt";
+import { loadSkill } from "../../utils/load-skill";
 import type { Octokits } from "../../github/api/client";
 import type { PrepareResult } from "../../prepare/types";
 
@@ -84,6 +85,8 @@ export async function prepareReviewMode({
     githubToken,
   });
 
+  const reviewSkillContent = await loadSkill("review");
+
   await createPrompt({
     githubContext: context,
     commentId,
@@ -95,6 +98,7 @@ export async function prepareReviewMode({
     },
     generatePrompt: generateReviewCandidatesPrompt,
     reviewArtifacts,
+    reviewSkillContent,
   });
   core.exportVariable("DROID_EXEC_RUN_TYPE", "droid-review");
 

src/utils/load-skill.ts

@@ -1,13 +1,16 @@
 import { readFile, readdir } from "fs/promises";
 import { resolve, join } from "path";
 
+const PLUGINS_REPO = "Factory-AI/factory-plugins";
+const PLUGINS_BRANCH = "main";
+
 const SHARED_BEGIN = "<!-- BEGIN_SHARED_METHODOLOGY -->";
 const SHARED_END = "<!-- END_SHARED_METHODOLOGY -->";
 
 /**
  * Format skill content for inclusion in a CI prompt.
- * Extracts only the shared methodology (between markers) so deep-specific
- * instructions in the CI template are authoritative for execution behavior.
+ * Extracts only the shared methodology (between markers) so CI-specific
+ * instructions in the template remain authoritative for execution behavior.
  */
 export function formatSkillSection(skillContent: string | undefined): string {
   if (!skillContent) return "";
@@ -34,38 +37,78 @@ export function extractSharedMethodology(content: string): string {
 }
 
 /**
- * Load a skill from the core plugin cache.
+ * Load a skill from the local core plugin cache.
  * The Droid CLI installs the core plugin to:
  *   ~/.factory/plugins/cache/factory-plugins/core/<hash>/skills/<name>/SKILL.md
  */
-export async function loadSkill(
+async function loadSkillFromCache(
   skillName: string,
 ): Promise<string | undefined> {
   const home = process.env.HOME || "~";
   const cacheDir = resolve(home, ".factory/plugins/cache/factory-plugins/core");
 
+  let entries: string[];
   try {
-    const entries = await readdir(cacheDir);
-    for (const hash of entries) {
-      const skillPath = join(cacheDir, hash, "skills", skillName, "SKILL.md");
-      try {
-        const content = await readFile(skillPath, "utf8");
-        const trimmed = content.trim();
-        if (!trimmed) continue;
-        console.log(
-          `Loaded skill ${skillName} from ${skillPath} (${trimmed.length} bytes)`,
-        );
-        return trimmed;
-      } catch {
-        continue;
-      }
+    entries = await readdir(cacheDir);
+  } catch {
+    return undefined;
+  }
+
+  for (const hash of entries) {
+    const skillPath = join(cacheDir, hash, "skills", skillName, "SKILL.md");
+    try {
+      const content = await readFile(skillPath, "utf8");
+      const trimmed = content.trim();
+      if (!trimmed) continue;
+      console.log(
+        `Loaded skill ${skillName} from ${skillPath} (${trimmed.length} bytes)`,
+      );
+      return trimmed;
+    } catch {
+      // SKILL.md not found under this hash entry, try next
+      continue;
     }
+  }
+
+  return undefined;
+}
+
+/**
+ * Fetch a skill from the factory-plugins GitHub repo.
+ * Used as fallback when the local plugin cache is not available (e.g. CI).
+ */
+async function loadSkillFromGitHub(
+  skillName: string,
+): Promise<string | undefined> {
+  const url = `https://raw.githubusercontent.com/${PLUGINS_REPO}/${PLUGINS_BRANCH}/plugins/core/skills/${skillName}/SKILL.md`;
+  try {
+    const response = await fetch(url);
+    if (!response.ok) return undefined;
+    const content = await response.text();
+    const trimmed = content.trim();
+    if (!trimmed) return undefined;
+    console.log(
+      `Loaded skill ${skillName} from GitHub (${trimmed.length} bytes)`,
+    );
+    return trimmed;
   } catch {
-    // Cache dir doesn't exist
+    return undefined;
   }
+}
+
+/**
+ * Load a skill by name. Tries the local plugin cache first,
+ * then falls back to fetching from the factory-plugins GitHub repo.
+ */
+export async function loadSkill(
+  skillName: string,
+): Promise<string | undefined> {
+  const cached = await loadSkillFromCache(skillName);
+  if (cached) return cached;
+
+  const remote = await loadSkillFromGitHub(skillName);
+  if (remote) return remote;
 
-  console.log(
-    `Skill ${skillName} not found in core plugin cache. Ensure the Droid CLI is installed.`,
-  );
+  console.log(`Skill ${skillName} not found locally or on GitHub.`);
   return undefined;
 }