Skip to content

Commit b2494b9

Browse files
Migrate review to skill-based architecture
- Review methodology lives in factory-plugins core plugin (code-review skill) - CI loads shared methodology from plugin cache at runtime - Skill uses BEGIN/END_SHARED_METHODOLOGY markers to separate: - Shared: bug patterns, analysis discipline, reporting gate, priorities, format - Shallow (TUI): local context detection + local findings output - Deep (CI): pre-computed artifacts, MCP tools, posting, validator flow - CI always posts comments (deep flow); TUI skill returns locally (shallow flow) - formatSkillSection extracts only shared methodology for CI prompt injection Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
1 parent ff0dc3a commit b2494b9

File tree

3 files changed

+79
-0
lines changed

3 files changed

+79
-0
lines changed

src/utils/load-skill.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { readFile, readdir } from "fs/promises";
2+
import { resolve, join } from "path";
3+
4+
const SHARED_BEGIN = "<!-- BEGIN_SHARED_METHODOLOGY -->";
5+
const SHARED_END = "<!-- END_SHARED_METHODOLOGY -->";
6+
7+
/**
8+
* Format skill content for inclusion in a CI prompt.
9+
* Extracts only the shared methodology (between markers) so deep-specific
10+
* instructions in the CI template are authoritative for execution behavior.
11+
*/
12+
export function formatSkillSection(skillContent: string | undefined): string {
13+
if (!skillContent) return "";
14+
const methodology = extractSharedMethodology(skillContent);
15+
return `
16+
<code_review_methodology>
17+
${methodology}
18+
</code_review_methodology>
19+
`;
20+
}
21+
22+
/**
23+
* Extract the shared methodology section from a skill's content.
24+
* Looks for BEGIN_SHARED_METHODOLOGY / END_SHARED_METHODOLOGY markers.
25+
* Returns the full content if markers are not found.
26+
*/
27+
export function extractSharedMethodology(content: string): string {
28+
const beginIdx = content.indexOf(SHARED_BEGIN);
29+
const endIdx = content.indexOf(SHARED_END);
30+
if (beginIdx === -1 || endIdx === -1 || endIdx <= beginIdx) {
31+
return content;
32+
}
33+
return content.slice(beginIdx + SHARED_BEGIN.length, endIdx).trim();
34+
}
35+
36+
/**
37+
* Load a skill from the core plugin cache.
38+
* The Droid CLI installs the core plugin to:
39+
* ~/.factory/plugins/cache/factory-plugins/core/<hash>/skills/<name>/SKILL.md
40+
*/
41+
export async function loadSkill(
42+
skillName: string,
43+
): Promise<string | undefined> {
44+
const home = process.env.HOME || "~";
45+
const cacheDir = resolve(home, ".factory/plugins/cache/factory-plugins/core");
46+
47+
try {
48+
const entries = await readdir(cacheDir);
49+
for (const hash of entries) {
50+
const skillPath = join(cacheDir, hash, "skills", skillName, "SKILL.md");
51+
try {
52+
const content = await readFile(skillPath, "utf8");
53+
const trimmed = content.trim();
54+
if (!trimmed) continue;
55+
console.log(
56+
`Loaded skill ${skillName} from ${skillPath} (${trimmed.length} bytes)`,
57+
);
58+
return trimmed;
59+
} catch {
60+
continue;
61+
}
62+
}
63+
} catch {
64+
// Cache dir doesn't exist
65+
}
66+
67+
console.log(
68+
`Skill ${skillName} not found in core plugin cache. Ensure the Droid CLI is installed.`,
69+
);
70+
return undefined;
71+
}

test/integration/review-flow.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import * as createInitial from "../../src/github/operations/comments/create-init
88
import * as mcpInstaller from "../../src/mcp/install-mcp-server";
99
import * as actorValidation from "../../src/github/validation/actor";
1010
import * as promptModule from "../../src/create-prompt";
11+
import * as skillLoader from "../../src/utils/load-skill";
1112
import * as core from "@actions/core";
1213
import * as childProcess from "node:child_process";
1314

@@ -23,6 +24,7 @@ describe("review command integration", () => {
2324
let exportVarSpy: ReturnType<typeof spyOn>;
2425
let promptSpy: ReturnType<typeof spyOn>;
2526
let execSyncSpy: ReturnType<typeof spyOn>;
27+
let loadSkillSpy: ReturnType<typeof spyOn>;
2628

2729
beforeEach(async () => {
2830
tmpDir = await mkdtemp(path.join(os.tmpdir(), "review-int-"));
@@ -39,6 +41,7 @@ describe("review command integration", () => {
3941
promptSpy = spyOn(promptModule, "createPrompt").mockResolvedValue();
4042
setOutputSpy = spyOn(core, "setOutput").mockImplementation(() => {});
4143
exportVarSpy = spyOn(core, "exportVariable").mockImplementation(() => {});
44+
loadSkillSpy = spyOn(skillLoader, "loadSkill").mockResolvedValue(undefined);
4245

4346
execSyncSpy = spyOn(childProcess, "execSync").mockImplementation(((
4447
cmd: string,
@@ -60,6 +63,7 @@ describe("review command integration", () => {
6063
setOutputSpy.mockRestore();
6164
exportVarSpy.mockRestore();
6265
execSyncSpy.mockRestore();
66+
loadSkillSpy.mockRestore();
6367

6468
if (process.env.RUNNER_TEMP) {
6569
await rm(process.env.RUNNER_TEMP, { recursive: true, force: true });

test/modes/tag/review-command.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { createMockContext } from "../../mockContext";
66
import * as promptModule from "../../../src/create-prompt";
77
import * as mcpInstaller from "../../../src/mcp/install-mcp-server";
88
import * as comments from "../../../src/github/operations/comments/create-initial";
9+
import * as skillLoader from "../../../src/utils/load-skill";
910
import * as childProcess from "child_process";
1011
import * as fsPromises from "fs/promises";
1112

@@ -39,6 +40,7 @@ describe("prepareReviewMode", () => {
3940
let execSyncSpy: ReturnType<typeof spyOn>;
4041
let writeFileSpy: ReturnType<typeof spyOn>;
4142
let mkdirSpy: ReturnType<typeof spyOn>;
43+
let loadSkillSpy: ReturnType<typeof spyOn>;
4244

4345
beforeEach(() => {
4446
process.env.DROID_ARGS = "";
@@ -57,6 +59,7 @@ describe("prepareReviewMode", () => {
5759
exportVariableSpy = spyOn(core, "exportVariable").mockImplementation(
5860
() => {},
5961
);
62+
loadSkillSpy = spyOn(skillLoader, "loadSkill").mockResolvedValue(undefined);
6063
// Mock execSync for git commands
6164
execSyncSpy = spyOn(childProcess, "execSync").mockImplementation(((
6265
cmd: string,
@@ -84,6 +87,7 @@ describe("prepareReviewMode", () => {
8487
execSyncSpy.mockRestore();
8588
writeFileSpy.mockRestore();
8689
mkdirSpy.mockRestore();
90+
loadSkillSpy.mockRestore();
8791
process.env.DROID_ARGS = originalArgs;
8892
if (originalReviewModel !== undefined) {
8993
process.env.REVIEW_MODEL = originalReviewModel;

0 commit comments

Comments
 (0)