fix(skills): register security skills for profiles
This commit is contained in:
@@ -1,15 +1,207 @@
|
||||
import { readFileSync, writeFileSync } from "node:fs"
|
||||
import { existsSync, readdirSync, readFileSync, writeFileSync } from "node:fs"
|
||||
import { basename, dirname, join } from "node:path"
|
||||
import type { ConfigMergeResult } from "../types"
|
||||
import { PLUGIN_NAME, LEGACY_PLUGIN_NAME } from "../../shared"
|
||||
import { backupConfigFile } from "./backup-config"
|
||||
import { getConfigDir } from "./config-context"
|
||||
import { ensureConfigDirectoryExists } from "./ensure-config-directory-exists"
|
||||
import { formatErrorWithSuggestion } from "./format-error-with-suggestion"
|
||||
import { detectConfigFormat } from "./opencode-config-format"
|
||||
import { detectConfigFormat, type ConfigFormat } from "./opencode-config-format"
|
||||
import { parseOpenCodeConfigFileWithError, type OpenCodeConfig } from "./parse-opencode-config-file"
|
||||
import { getPluginNameWithVersion } from "./plugin-name-with-version"
|
||||
import { checkVersionCompatibility, extractVersionFromPluginEntry } from "./version-compatibility"
|
||||
|
||||
type ConfigTarget = {
|
||||
readonly format: ConfigFormat
|
||||
readonly path: string
|
||||
readonly primary: boolean
|
||||
}
|
||||
|
||||
function detectConfigFormatInDir(configDir: string): { readonly format: ConfigFormat; readonly path: string } {
|
||||
const configJsonc = join(configDir, "opencode.jsonc")
|
||||
const configJson = join(configDir, "opencode.json")
|
||||
|
||||
if (existsSync(configJsonc)) {
|
||||
return { format: "jsonc", path: configJsonc }
|
||||
}
|
||||
if (existsSync(configJson)) {
|
||||
return { format: "json", path: configJson }
|
||||
}
|
||||
return { format: "none", path: configJson }
|
||||
}
|
||||
|
||||
function getParentConfigDirForProfile(configDir: string): string | null {
|
||||
const parentDir = dirname(configDir)
|
||||
if (basename(parentDir) !== "profiles") return null
|
||||
return dirname(parentDir)
|
||||
}
|
||||
|
||||
function listProfileConfigDirs(rootConfigDir: string): string[] {
|
||||
const profilesDir = join(rootConfigDir, "profiles")
|
||||
if (!existsSync(profilesDir)) return []
|
||||
|
||||
return readdirSync(profilesDir, { withFileTypes: true })
|
||||
.filter((entry) => entry.isDirectory())
|
||||
.map((entry) => join(profilesDir, entry.name))
|
||||
.filter((profileDir) => detectConfigFormatInDir(profileDir).format !== "none")
|
||||
}
|
||||
|
||||
function getConfigTargets(): ConfigTarget[] {
|
||||
const primaryConfigDir = getConfigDir()
|
||||
const rootConfigDir = getParentConfigDirForProfile(primaryConfigDir) ?? primaryConfigDir
|
||||
const targetDirs = new Set<string>([primaryConfigDir])
|
||||
|
||||
if (rootConfigDir !== primaryConfigDir && detectConfigFormatInDir(rootConfigDir).format !== "none") {
|
||||
targetDirs.add(rootConfigDir)
|
||||
}
|
||||
|
||||
for (const profileConfigDir of listProfileConfigDirs(rootConfigDir)) {
|
||||
targetDirs.add(profileConfigDir)
|
||||
}
|
||||
|
||||
return Array.from(targetDirs).map((configDir) => {
|
||||
const detected = detectConfigFormatInDir(configDir)
|
||||
return {
|
||||
...detected,
|
||||
primary: configDir === primaryConfigDir,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
function isSourceOmoPluginEntry(plugin: string): boolean {
|
||||
const normalized = plugin.toLowerCase().replaceAll("\\", "/")
|
||||
if (!normalized.startsWith("file://")) return false
|
||||
|
||||
return /\/(omo(?:-[^/]*)?|oh-my-opencode|oh-my-openagent)\/(src|dist)\/index\.(ts|js)$/.test(normalized)
|
||||
}
|
||||
|
||||
function isPackageOmoPluginEntry(plugin: string): boolean {
|
||||
return plugin === PLUGIN_NAME || plugin.startsWith(`${PLUGIN_NAME}@`) ||
|
||||
plugin === LEGACY_PLUGIN_NAME || plugin.startsWith(`${LEGACY_PLUGIN_NAME}@`)
|
||||
}
|
||||
|
||||
function isOurPlugin(plugin: string): boolean {
|
||||
return isPackageOmoPluginEntry(plugin) || isSourceOmoPluginEntry(plugin)
|
||||
}
|
||||
|
||||
function findOurPluginEntry(plugins: readonly string[]): string | undefined {
|
||||
return plugins.find(isOurPlugin)
|
||||
}
|
||||
|
||||
function findSourcePluginEntryInTarget(target: ConfigTarget): string | null {
|
||||
if (target.format === "none") return null
|
||||
|
||||
const parseResult = parseOpenCodeConfigFileWithError(target.path)
|
||||
const plugins = parseResult.config?.plugin ?? []
|
||||
return plugins.find(isSourceOmoPluginEntry) ?? null
|
||||
}
|
||||
|
||||
function choosePluginEntry(params: {
|
||||
readonly existingEntry: string | undefined
|
||||
readonly fallbackEntry: string
|
||||
readonly preferredSourceEntry: string | null
|
||||
}): string {
|
||||
if (params.existingEntry && isSourceOmoPluginEntry(params.existingEntry)) {
|
||||
return params.existingEntry
|
||||
}
|
||||
if (params.preferredSourceEntry) {
|
||||
return params.preferredSourceEntry
|
||||
}
|
||||
return params.fallbackEntry
|
||||
}
|
||||
|
||||
function writePluginEntryToTarget(params: {
|
||||
readonly target: ConfigTarget
|
||||
readonly currentVersion: string
|
||||
readonly fallbackEntry: string
|
||||
readonly preferredSourceEntry: string | null
|
||||
}): ConfigMergeResult {
|
||||
const { target, currentVersion, fallbackEntry, preferredSourceEntry } = params
|
||||
const pluginEntry = choosePluginEntry({
|
||||
existingEntry: undefined,
|
||||
fallbackEntry,
|
||||
preferredSourceEntry,
|
||||
})
|
||||
|
||||
try {
|
||||
if (target.format === "none") {
|
||||
const config: OpenCodeConfig = { plugin: [pluginEntry] }
|
||||
writeFileSync(target.path, JSON.stringify(config, null, 2) + "\n")
|
||||
return { success: true, configPath: target.path }
|
||||
}
|
||||
|
||||
const parseResult = parseOpenCodeConfigFileWithError(target.path)
|
||||
if (!parseResult.config) {
|
||||
return {
|
||||
success: false,
|
||||
configPath: target.path,
|
||||
error: parseResult.error ?? "Failed to parse config file",
|
||||
}
|
||||
}
|
||||
|
||||
const config = parseResult.config
|
||||
const plugins = config.plugin ?? []
|
||||
const existingEntry = findOurPluginEntry(plugins)
|
||||
const nextPluginEntry = choosePluginEntry({
|
||||
existingEntry,
|
||||
fallbackEntry,
|
||||
preferredSourceEntry,
|
||||
})
|
||||
|
||||
if (existingEntry && !preferredSourceEntry) {
|
||||
const installedVersion = extractVersionFromPluginEntry(existingEntry)
|
||||
const compatibility = checkVersionCompatibility(installedVersion, currentVersion)
|
||||
|
||||
if (!compatibility.canUpgrade) {
|
||||
return {
|
||||
success: false,
|
||||
configPath: target.path,
|
||||
error: compatibility.reason ?? "Version compatibility check failed",
|
||||
}
|
||||
}
|
||||
|
||||
const backupResult = backupConfigFile(target.path)
|
||||
if (!backupResult.success) {
|
||||
return {
|
||||
success: false,
|
||||
configPath: target.path,
|
||||
error: `Failed to create backup: ${backupResult.error}`,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const normalizedPlugins = plugins.filter((plugin) => !isOurPlugin(plugin))
|
||||
normalizedPlugins.push(nextPluginEntry)
|
||||
|
||||
config.plugin = normalizedPlugins
|
||||
|
||||
if (target.format === "jsonc") {
|
||||
const content = readFileSync(target.path, "utf-8")
|
||||
const pluginArrayRegex = /((?:"plugin"|plugin)\s*:\s*)\[([\s\S]*?)\]/
|
||||
const match = content.match(pluginArrayRegex)
|
||||
|
||||
if (match) {
|
||||
const formattedPlugins = normalizedPlugins.map((p) => `"${p}"`).join(",\n ")
|
||||
const newContent = content.replace(pluginArrayRegex, `$1[\n ${formattedPlugins}\n ]`)
|
||||
writeFileSync(target.path, newContent)
|
||||
} else {
|
||||
const newContent = content.replace(/(\{)/, `$1\n "plugin": ["${nextPluginEntry}"],`)
|
||||
writeFileSync(target.path, newContent)
|
||||
}
|
||||
} else {
|
||||
writeFileSync(target.path, JSON.stringify(config, null, 2) + "\n")
|
||||
}
|
||||
|
||||
return { success: true, configPath: target.path }
|
||||
} catch (err) {
|
||||
return {
|
||||
success: false,
|
||||
configPath: target.path,
|
||||
error: formatErrorWithSuggestion(err, "update opencode config"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export async function addPluginToOpenCodeConfig(currentVersion: string): Promise<ConfigMergeResult> {
|
||||
try {
|
||||
ensureConfigDirectoryExists()
|
||||
@@ -21,91 +213,27 @@ export async function addPluginToOpenCodeConfig(currentVersion: string): Promise
|
||||
}
|
||||
}
|
||||
|
||||
const { format, path } = detectConfigFormat()
|
||||
const primaryTarget = detectConfigFormat()
|
||||
const targets = getConfigTargets()
|
||||
const preferredSourceEntry = targets
|
||||
.map((target) => findSourcePluginEntryInTarget(target))
|
||||
.find((entry): entry is string => entry !== null) ?? null
|
||||
const pluginEntry = await getPluginNameWithVersion(currentVersion, PLUGIN_NAME)
|
||||
|
||||
try {
|
||||
if (format === "none") {
|
||||
const config: OpenCodeConfig = { plugin: [pluginEntry] }
|
||||
writeFileSync(path, JSON.stringify(config, null, 2) + "\n")
|
||||
return { success: true, configPath: path }
|
||||
}
|
||||
let primaryResult: ConfigMergeResult | null = null
|
||||
for (const target of targets) {
|
||||
const result = writePluginEntryToTarget({
|
||||
target,
|
||||
currentVersion,
|
||||
fallbackEntry: pluginEntry,
|
||||
preferredSourceEntry,
|
||||
})
|
||||
|
||||
const parseResult = parseOpenCodeConfigFileWithError(path)
|
||||
if (!parseResult.config) {
|
||||
return {
|
||||
success: false,
|
||||
configPath: path,
|
||||
error: parseResult.error ?? "Failed to parse config file",
|
||||
}
|
||||
}
|
||||
|
||||
const config = parseResult.config
|
||||
const plugins = config.plugin ?? []
|
||||
|
||||
const canonicalEntries = plugins.filter(
|
||||
(plugin) => plugin === PLUGIN_NAME || plugin.startsWith(`${PLUGIN_NAME}@`)
|
||||
)
|
||||
const legacyEntries = plugins.filter(
|
||||
(plugin) => plugin === LEGACY_PLUGIN_NAME || plugin.startsWith(`${LEGACY_PLUGIN_NAME}@`)
|
||||
)
|
||||
const otherPlugins = plugins.filter(
|
||||
(plugin) => !(plugin === PLUGIN_NAME || plugin.startsWith(`${PLUGIN_NAME}@`))
|
||||
&& !(plugin === LEGACY_PLUGIN_NAME || plugin.startsWith(`${LEGACY_PLUGIN_NAME}@`))
|
||||
)
|
||||
|
||||
const existingEntry = canonicalEntries[0] ?? legacyEntries[0]
|
||||
if (existingEntry) {
|
||||
const installedVersion = extractVersionFromPluginEntry(existingEntry)
|
||||
const compatibility = checkVersionCompatibility(installedVersion, currentVersion)
|
||||
|
||||
if (!compatibility.canUpgrade) {
|
||||
return {
|
||||
success: false,
|
||||
configPath: path,
|
||||
error: compatibility.reason ?? "Version compatibility check failed",
|
||||
}
|
||||
}
|
||||
|
||||
const backupResult = backupConfigFile(path)
|
||||
if (!backupResult.success) {
|
||||
return {
|
||||
success: false,
|
||||
configPath: path,
|
||||
error: `Failed to create backup: ${backupResult.error}`,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const normalizedPlugins = [...otherPlugins]
|
||||
|
||||
normalizedPlugins.push(pluginEntry)
|
||||
|
||||
config.plugin = normalizedPlugins
|
||||
|
||||
if (format === "jsonc") {
|
||||
const content = readFileSync(path, "utf-8")
|
||||
const pluginArrayRegex = /((?:"plugin"|plugin)\s*:\s*)\[([\s\S]*?)\]/
|
||||
const match = content.match(pluginArrayRegex)
|
||||
|
||||
if (match) {
|
||||
const formattedPlugins = normalizedPlugins.map((p) => `"${p}"`).join(",\n ")
|
||||
const newContent = content.replace(pluginArrayRegex, `$1[\n ${formattedPlugins}\n ]`)
|
||||
writeFileSync(path, newContent)
|
||||
} else {
|
||||
const newContent = content.replace(/(\{)/, `$1\n "plugin": ["${pluginEntry}"],`)
|
||||
writeFileSync(path, newContent)
|
||||
}
|
||||
} else {
|
||||
writeFileSync(path, JSON.stringify(config, null, 2) + "\n")
|
||||
}
|
||||
|
||||
return { success: true, configPath: path }
|
||||
} catch (err) {
|
||||
return {
|
||||
success: false,
|
||||
configPath: path,
|
||||
error: formatErrorWithSuggestion(err, "update opencode config"),
|
||||
if (!result.success) return result
|
||||
if (target.primary) {
|
||||
primaryResult = result
|
||||
}
|
||||
}
|
||||
|
||||
return primaryResult ?? { success: true, configPath: primaryTarget.path }
|
||||
}
|
||||
|
||||
@@ -184,4 +184,56 @@ describe("addPluginToOpenCodeConfig - single package writes", () => {
|
||||
expect(savedContent.includes('"plugin": [\n "oh-my-openagent"\n ]')).toBe(true)
|
||||
expect(savedContent.includes("oh-my-opencode")).toBe(false)
|
||||
})
|
||||
|
||||
it("mirrors an existing source plugin entry into profile configs", async () => {
|
||||
// given
|
||||
const sourcePlugin = "file:///Users/yeongyu/local-workspaces/omo/src/index.ts"
|
||||
writeFileSync(testConfigPath, JSON.stringify({ plugin: [sourcePlugin] }, null, 2) + "\n", "utf-8")
|
||||
|
||||
const profileDir = join(testConfigDir, "profiles", "today")
|
||||
const profileConfigPath = join(profileDir, "opencode.json")
|
||||
mkdirSync(profileDir, { recursive: true })
|
||||
writeFileSync(
|
||||
profileConfigPath,
|
||||
JSON.stringify({ $schema: "https://opencode.ai/config.json" }, null, 2) + "\n",
|
||||
"utf-8",
|
||||
)
|
||||
|
||||
// when
|
||||
const result = await addPluginToOpenCodeConfig("3.11.0")
|
||||
|
||||
// then
|
||||
expect(result.success).toBe(true)
|
||||
const savedRootConfig = JSON.parse(readFileSync(testConfigPath, "utf-8"))
|
||||
const savedProfileConfig = JSON.parse(readFileSync(profileConfigPath, "utf-8"))
|
||||
expect(savedRootConfig.plugin).toEqual([sourcePlugin])
|
||||
expect(savedProfileConfig.plugin).toEqual([sourcePlugin])
|
||||
})
|
||||
|
||||
it("uses the parent source plugin entry when OPENCODE_CONFIG_DIR points at a profile", async () => {
|
||||
// given
|
||||
const sourcePlugin = "file:///Users/yeongyu/local-workspaces/omo/src/index.ts"
|
||||
writeFileSync(testConfigPath, JSON.stringify({ plugin: [sourcePlugin] }, null, 2) + "\n", "utf-8")
|
||||
|
||||
const profileDir = join(testConfigDir, "profiles", "today")
|
||||
const profileConfigPath = join(profileDir, "opencode.json")
|
||||
mkdirSync(profileDir, { recursive: true })
|
||||
writeFileSync(
|
||||
profileConfigPath,
|
||||
JSON.stringify({ $schema: "https://opencode.ai/config.json" }, null, 2) + "\n",
|
||||
"utf-8",
|
||||
)
|
||||
|
||||
process.env.OPENCODE_CONFIG_DIR = profileDir
|
||||
resetConfigContext()
|
||||
|
||||
// when
|
||||
const result = await addPluginToOpenCodeConfig("3.11.0")
|
||||
|
||||
// then
|
||||
expect(result.success).toBe(true)
|
||||
expect(result.configPath.endsWith("/profiles/today/opencode.json")).toBe(true)
|
||||
const savedProfileConfig = JSON.parse(readFileSync(profileConfigPath, "utf-8"))
|
||||
expect(savedProfileConfig.plugin).toEqual([sourcePlugin])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -249,6 +249,7 @@ describe("createBuiltinSkills", () => {
|
||||
|
||||
// #then
|
||||
expect(securityReview?.description).toContain("Alias for security-research")
|
||||
expect(securityReview?.description).toContain("/security-review")
|
||||
expect(securityReview?.template).toBe(securityResearch?.template)
|
||||
})
|
||||
|
||||
|
||||
@@ -3,6 +3,6 @@ import { securityResearchSkill } from "./security-research"
|
||||
|
||||
export const securityReviewSkill: BuiltinSkill = {
|
||||
name: "security-review",
|
||||
description: `Alias for security-research. ${securityResearchSkill.description}`,
|
||||
description: `Alias for security-research and /security-review. ${securityResearchSkill.description}`,
|
||||
template: securityResearchSkill.template,
|
||||
}
|
||||
|
||||
@@ -19,6 +19,75 @@ describe("createSkillContext", () => {
|
||||
rmSync(testDirectory, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it("exposes security skills to the OMO skill tool context", async () => {
|
||||
// given
|
||||
const discoverConfigSourceSkillsSpy = spyOn(
|
||||
skillLoader,
|
||||
"discoverConfigSourceSkills",
|
||||
).mockResolvedValue([])
|
||||
const discoverUserClaudeSkillsSpy = spyOn(
|
||||
skillLoader,
|
||||
"discoverUserClaudeSkills",
|
||||
).mockResolvedValue([])
|
||||
const discoverProjectClaudeSkillsSpy = spyOn(
|
||||
skillLoader,
|
||||
"discoverProjectClaudeSkills",
|
||||
).mockResolvedValue([])
|
||||
const discoverOpencodeGlobalSkillsSpy = spyOn(
|
||||
skillLoader,
|
||||
"discoverOpencodeGlobalSkills",
|
||||
).mockResolvedValue([])
|
||||
const discoverOpencodeProjectSkillsSpy = spyOn(
|
||||
skillLoader,
|
||||
"discoverOpencodeProjectSkills",
|
||||
).mockResolvedValue([])
|
||||
const discoverProjectAgentsSkillsSpy = spyOn(
|
||||
skillLoader,
|
||||
"discoverProjectAgentsSkills",
|
||||
).mockResolvedValue([])
|
||||
const discoverGlobalAgentsSkillsSpy = spyOn(
|
||||
skillLoader,
|
||||
"discoverGlobalAgentsSkills",
|
||||
).mockResolvedValue([])
|
||||
const getSystemMcpServerNamesSpy = spyOn(
|
||||
mcpLoader,
|
||||
"getSystemMcpServerNames",
|
||||
).mockReturnValue(new Set<string>())
|
||||
|
||||
const pluginConfig = OhMyOpenCodeConfigSchema.parse({})
|
||||
|
||||
try {
|
||||
// when
|
||||
const result = await createSkillContext({
|
||||
directory: testDirectory,
|
||||
pluginConfig,
|
||||
})
|
||||
|
||||
// then
|
||||
expect(result.mergedSkills.some((skill) => skill.name === "security-research")).toBe(true)
|
||||
expect(result.mergedSkills.some((skill) => skill.name === "security-review")).toBe(true)
|
||||
expect(result.availableSkills).toContainEqual({
|
||||
name: "security-research",
|
||||
description: expect.stringContaining("security research"),
|
||||
location: "plugin",
|
||||
})
|
||||
expect(result.availableSkills).toContainEqual({
|
||||
name: "security-review",
|
||||
description: expect.stringContaining("/security-review"),
|
||||
location: "plugin",
|
||||
})
|
||||
} finally {
|
||||
discoverConfigSourceSkillsSpy.mockRestore()
|
||||
discoverUserClaudeSkillsSpy.mockRestore()
|
||||
discoverProjectClaudeSkillsSpy.mockRestore()
|
||||
discoverOpencodeGlobalSkillsSpy.mockRestore()
|
||||
discoverOpencodeProjectSkillsSpy.mockRestore()
|
||||
discoverProjectAgentsSkillsSpy.mockRestore()
|
||||
discoverGlobalAgentsSkillsSpy.mockRestore()
|
||||
getSystemMcpServerNamesSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it("excludes discovered playwright skill when browser provider is agent-browser", async () => {
|
||||
// given
|
||||
const discoveredPlaywrightDir = join(testDirectory, ".claude", "skills", "playwright")
|
||||
|
||||
@@ -834,6 +834,31 @@ describe("skill tool - nativeSkills integration", () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe("skill tool - bundled security skills", () => {
|
||||
it("loads security-research and security-review when the plugin skill context pre-seeds them", async () => {
|
||||
//#given
|
||||
const { builtinToLoadedSkill } = await import("../../../features/opencode-skill-loader/merger/builtin-skill-converter")
|
||||
const { securityResearchSkill, securityReviewSkill } = await import("../../../features/builtin-skills/skills/index")
|
||||
const tool = createSkillTool({
|
||||
directory: "/test",
|
||||
skills: [
|
||||
builtinToLoadedSkill(securityResearchSkill),
|
||||
builtinToLoadedSkill(securityReviewSkill),
|
||||
],
|
||||
})
|
||||
|
||||
//#when
|
||||
const researchResult = await tool.execute({ name: "security-research" }, mockContext)
|
||||
const reviewResult = await tool.execute({ name: "security-review" }, mockContext)
|
||||
|
||||
//#then
|
||||
expect(researchResult).toContain("## Skill: security-research")
|
||||
expect(researchResult).toContain("Security Research - Team Mode Vulnerability Audit")
|
||||
expect(reviewResult).toContain("## Skill: security-review")
|
||||
expect(reviewResult).toContain("Security Research - Team Mode Vulnerability Audit")
|
||||
})
|
||||
})
|
||||
|
||||
describe("skill tool - short name resolution", () => {
|
||||
it("resolves namespaced skill by short name when unambiguous", async () => {
|
||||
// given
|
||||
|
||||
Reference in New Issue
Block a user