From e36385e671511b5dfb5ff99e75ed582d78b2a8de Mon Sep 17 00:00:00 2001 From: Nguyen Khac Trung Kien Date: Fri, 16 Jan 2026 08:01:04 +0700 Subject: [PATCH] fix(git-master): inject watermark only when enabled instead of overriding defaults The watermark (commit footer and co-author) was inconsistently applied because: 1. The skill tool didn't receive gitMasterConfig 2. The approach was 'default ON, inject DISABLED override' which LLMs sometimes ignored This refactors to 'inject only when enabled' approach: - Remove hardcoded watermark section from base templates - Dynamically inject section 5.5 based on config values - Default is still ON (both true when no config) - When both disabled, no injection occurs (clean prompt) Also fixes missing config propagation to skill tool and createBuiltinAgents. --- src/agents/utils.ts | 12 +-- .../builtin-skills/git-master/SKILL.md | 27 ------ src/features/builtin-skills/skills.ts | 29 +------ .../skill-content.test.ts | 82 +++++++++++++++++-- .../opencode-skill-loader/skill-content.ts | 78 +++++++++++++----- src/index.ts | 1 + src/plugin-handlers/config-handler.ts | 3 +- src/tools/skill/tools.ts | 8 +- src/tools/skill/types.ts | 3 + 9 files changed, 157 insertions(+), 86 deletions(-) diff --git a/src/agents/utils.ts b/src/agents/utils.ts index d831caa87..7bf98a51a 100644 --- a/src/agents/utils.ts +++ b/src/agents/utils.ts @@ -1,6 +1,6 @@ import type { AgentConfig } from "@opencode-ai/sdk" import type { BuiltinAgentName, AgentOverrideConfig, AgentOverrides, AgentFactory, AgentPromptMetadata } from "./types" -import type { CategoriesConfig, CategoryConfig } from "../config/schema" +import type { CategoriesConfig, CategoryConfig, GitMasterConfig } from "../config/schema" import { createSisyphusAgent } from "./sisyphus" import { createOracleAgent, ORACLE_PROMPT_METADATA } from "./oracle" import { createLibrarianAgent, LIBRARIAN_PROMPT_METADATA } from "./librarian" @@ -51,7 +51,8 @@ function isFactory(source: AgentSource): source is AgentFactory { export function buildAgent( source: AgentSource, model?: string, - categories?: CategoriesConfig + categories?: CategoriesConfig, + gitMasterConfig?: GitMasterConfig ): AgentConfig { const base = isFactory(source) ? source(model) : source const categoryConfigs: Record = categories @@ -75,7 +76,7 @@ export function buildAgent( } if (agentWithCategory.skills?.length) { - const { resolved } = resolveMultipleSkills(agentWithCategory.skills) + const { resolved } = resolveMultipleSkills(agentWithCategory.skills, { gitMasterConfig }) if (resolved.size > 0) { const skillContent = Array.from(resolved.values()).join("\n\n") base.prompt = skillContent + (base.prompt ? "\n\n" + base.prompt : "") @@ -130,7 +131,8 @@ export function createBuiltinAgents( agentOverrides: AgentOverrides = {}, directory?: string, systemDefaultModel?: string, - categories?: CategoriesConfig + categories?: CategoriesConfig, + gitMasterConfig?: GitMasterConfig ): Record { const result: Record = {} const availableAgents: AvailableAgent[] = [] @@ -149,7 +151,7 @@ export function createBuiltinAgents( const override = agentOverrides[agentName] const model = override?.model - let config = buildAgent(source, model, mergedCategories) + let config = buildAgent(source, model, mergedCategories, gitMasterConfig) if (agentName === "librarian" && directory && config.prompt) { const envContext = createEnvContext() diff --git a/src/features/builtin-skills/git-master/SKILL.md b/src/features/builtin-skills/git-master/SKILL.md index 14566c0ee..d1baece0b 100644 --- a/src/features/builtin-skills/git-master/SKILL.md +++ b/src/features/builtin-skills/git-master/SKILL.md @@ -529,33 +529,6 @@ IF style == SHORT: 3. Is it similar to examples from git log? If ANY check fails -> REWRITE message. - -### 5.5 Commit Footer & Co-Author (Configurable) - -**Check oh-my-opencode.json for these flags:** -- `git_master.commit_footer` (default: true) - adds footer message -- `git_master.include_co_authored_by` (default: true) - adds co-author trailer - -If enabled, add Sisyphus attribution to EVERY commit: - -1. **Footer in commit body (if `commit_footer: true`):** -``` -Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) -``` - -2. **Co-authored-by trailer (if `include_co_authored_by: true`):** -``` -Co-authored-by: Sisyphus -``` - -**Example (both enabled):** -```bash -git commit -m "{Commit Message}" -m "Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)" -m "Co-authored-by: Sisyphus " -``` - -**To disable:** Set in oh-my-opencode.json: -```json -{ "git_master": { "commit_footer": false, "include_co_authored_by": false } } ``` diff --git a/src/features/builtin-skills/skills.ts b/src/features/builtin-skills/skills.ts index 6106a98f1..cde373217 100644 --- a/src/features/builtin-skills/skills.ts +++ b/src/features/builtin-skills/skills.ts @@ -622,35 +622,8 @@ IF style == SHORT: 3. Is it similar to examples from git log? If ANY check fails -> REWRITE message. - -### 5.5 Commit Footer & Co-Author (Configurable) - -**Check oh-my-opencode.json for these flags:** -- \`git_master.commit_footer\` (default: true) - adds footer message -- \`git_master.include_co_authored_by\` (default: true) - adds co-author trailer - -If enabled, add Sisyphus attribution to EVERY commit: - -1. **Footer in commit body (if \`commit_footer: true\`):** \`\`\` -Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) -\`\`\` - -2. **Co-authored-by trailer (if \`include_co_authored_by: true\`):** -\`\`\` -Co-authored-by: Sisyphus -\`\`\` - -**Example (both enabled):** -\`\`\`bash -git commit -m "{Commit Message}" -m "Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)" -m "Co-authored-by: Sisyphus " -\`\`\` - -**To disable:** Set in oh-my-opencode.json: -\`\`\`json -{ "git_master": { "commit_footer": false, "include_co_authored_by": false } } -\`\`\` - +\ --- diff --git a/src/features/opencode-skill-loader/skill-content.test.ts b/src/features/opencode-skill-loader/skill-content.test.ts index 23f455ccd..fd8c597da 100644 --- a/src/features/opencode-skill-loader/skill-content.test.ts +++ b/src/features/opencode-skill-loader/skill-content.test.ts @@ -160,8 +160,8 @@ describe("resolveMultipleSkillsAsync", () => { expect(result.resolved.get("playwright")).toContain("Playwright Browser Automation") }) - it("should support git-master config injection", async () => { - // #given: git-master skill with config override + it("should NOT inject watermark when both options are disabled", async () => { + // #given: git-master skill with watermark disabled const skillNames = ["git-master"] const options = { gitMasterConfig: { @@ -173,12 +173,84 @@ describe("resolveMultipleSkillsAsync", () => { // #when: resolving with git-master config const result = await resolveMultipleSkillsAsync(skillNames, options) - // #then: config values injected into template + // #then: no watermark section injected expect(result.resolved.size).toBe(1) expect(result.notFound).toEqual([]) const gitMasterContent = result.resolved.get("git-master") - expect(gitMasterContent).toContain("commit_footer") - expect(gitMasterContent).toContain("DISABLED") + expect(gitMasterContent).not.toContain("Ultraworked with") + expect(gitMasterContent).not.toContain("Co-authored-by: Sisyphus") + }) + + it("should inject watermark when enabled (default)", async () => { + // #given: git-master skill with default config (watermark enabled) + const skillNames = ["git-master"] + const options = { + gitMasterConfig: { + commit_footer: true, + include_co_authored_by: true, + }, + } + + // #when: resolving with git-master config + const result = await resolveMultipleSkillsAsync(skillNames, options) + + // #then: watermark section is injected + expect(result.resolved.size).toBe(1) + const gitMasterContent = result.resolved.get("git-master") + expect(gitMasterContent).toContain("Ultraworked with [Sisyphus]") + expect(gitMasterContent).toContain("Co-authored-by: Sisyphus") + }) + + it("should inject only footer when co-author is disabled", async () => { + // #given: git-master skill with only footer enabled + const skillNames = ["git-master"] + const options = { + gitMasterConfig: { + commit_footer: true, + include_co_authored_by: false, + }, + } + + // #when: resolving with git-master config + const result = await resolveMultipleSkillsAsync(skillNames, options) + + // #then: only footer is injected + const gitMasterContent = result.resolved.get("git-master") + expect(gitMasterContent).toContain("Ultraworked with [Sisyphus]") + expect(gitMasterContent).not.toContain("Co-authored-by: Sisyphus") + }) + + it("should inject watermark by default when no config provided", async () => { + // #given: git-master skill with NO config (default behavior) + const skillNames = ["git-master"] + + // #when: resolving without any gitMasterConfig + const result = await resolveMultipleSkillsAsync(skillNames) + + // #then: watermark is injected (default is ON) + expect(result.resolved.size).toBe(1) + const gitMasterContent = result.resolved.get("git-master") + expect(gitMasterContent).toContain("Ultraworked with [Sisyphus]") + expect(gitMasterContent).toContain("Co-authored-by: Sisyphus") + }) + + it("should inject only co-author when footer is disabled", async () => { + // #given: git-master skill with only co-author enabled + const skillNames = ["git-master"] + const options = { + gitMasterConfig: { + commit_footer: false, + include_co_authored_by: true, + }, + } + + // #when: resolving with git-master config + const result = await resolveMultipleSkillsAsync(skillNames, options) + + // #then: only co-author is injected + const gitMasterContent = result.resolved.get("git-master") + expect(gitMasterContent).not.toContain("Ultraworked with [Sisyphus]") + expect(gitMasterContent).toContain("Co-authored-by: Sisyphus") }) it("should handle empty array", async () => { diff --git a/src/features/opencode-skill-loader/skill-content.ts b/src/features/opencode-skill-loader/skill-content.ts index 98cf65cfe..182947508 100644 --- a/src/features/opencode-skill-loader/skill-content.ts +++ b/src/features/opencode-skill-loader/skill-content.ts @@ -59,22 +59,62 @@ async function extractSkillTemplate(skill: LoadedSkill): Promise { export { clearSkillCache, getAllSkills, extractSkillTemplate } -function injectGitMasterConfig(template: string, config?: GitMasterConfig): string { - if (!config) return template +export function injectGitMasterConfig(template: string, config?: GitMasterConfig): string { + const commitFooter = config?.commit_footer ?? true + const includeCoAuthoredBy = config?.include_co_authored_by ?? true - const commitFooter = config.commit_footer ?? true - const includeCoAuthoredBy = config.include_co_authored_by ?? true + if (!commitFooter && !includeCoAuthoredBy) { + return template + } - const configHeader = `## Git Master Configuration (from oh-my-opencode.json) + const sections: string[] = [] -**IMPORTANT: These values override the defaults in section 5.5:** -- \`commit_footer\`: ${commitFooter} ${!commitFooter ? "(DISABLED - do NOT add footer)" : ""} -- \`include_co_authored_by\`: ${includeCoAuthoredBy} ${!includeCoAuthoredBy ? "(DISABLED - do NOT add Co-authored-by)" : ""} + sections.push(`### 5.5 Commit Footer & Co-Author`) + sections.push(``) + sections.push(`Add Sisyphus attribution to EVERY commit:`) + sections.push(``) ---- + if (commitFooter) { + sections.push(`1. **Footer in commit body:**`) + sections.push("```") + sections.push(`Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)`) + sections.push("```") + sections.push(``) + } -` - return configHeader + template + if (includeCoAuthoredBy) { + sections.push(`${commitFooter ? "2" : "1"}. **Co-authored-by trailer:**`) + sections.push("```") + sections.push(`Co-authored-by: Sisyphus `) + sections.push("```") + sections.push(``) + } + + if (commitFooter && includeCoAuthoredBy) { + sections.push(`**Example (both enabled):**`) + sections.push("```bash") + sections.push(`git commit -m "{Commit Message}" -m "Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)" -m "Co-authored-by: Sisyphus "`) + sections.push("```") + } else if (commitFooter) { + sections.push(`**Example:**`) + sections.push("```bash") + sections.push(`git commit -m "{Commit Message}" -m "Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)"`) + sections.push("```") + } else if (includeCoAuthoredBy) { + sections.push(`**Example:**`) + sections.push("```bash") + sections.push(`git commit -m "{Commit Message}" -m "Co-authored-by: Sisyphus "`) + sections.push("```") + } + + const injection = sections.join("\n") + + const insertionPoint = template.indexOf("```\n") + if (insertionPoint !== -1) { + return template.slice(0, insertionPoint) + "```\n\n" + injection + "\n" + template.slice(insertionPoint + "```\n".length) + } + + return template + "\n\n" + injection } export function resolveSkillContent(skillName: string, options?: SkillResolutionOptions): string | null { @@ -82,8 +122,8 @@ export function resolveSkillContent(skillName: string, options?: SkillResolution const skill = skills.find((s) => s.name === skillName) if (!skill) return null - if (skillName === "git-master" && options?.gitMasterConfig) { - return injectGitMasterConfig(skill.template, options.gitMasterConfig) + if (skillName === "git-master") { + return injectGitMasterConfig(skill.template, options?.gitMasterConfig) } return skill.template @@ -102,8 +142,8 @@ export function resolveMultipleSkills(skillNames: string[], options?: SkillResol for (const name of skillNames) { const template = skillMap.get(name) if (template) { - if (name === "git-master" && options?.gitMasterConfig) { - resolved.set(name, injectGitMasterConfig(template, options.gitMasterConfig)) + if (name === "git-master") { + resolved.set(name, injectGitMasterConfig(template, options?.gitMasterConfig)) } else { resolved.set(name, template) } @@ -125,8 +165,8 @@ export async function resolveSkillContentAsync( const template = await extractSkillTemplate(skill) - if (skillName === "git-master" && options?.gitMasterConfig) { - return injectGitMasterConfig(template, options.gitMasterConfig) + if (skillName === "git-master") { + return injectGitMasterConfig(template, options?.gitMasterConfig) } return template @@ -152,8 +192,8 @@ export async function resolveMultipleSkillsAsync( const skill = skillMap.get(name) if (skill) { const template = await extractSkillTemplate(skill) - if (name === "git-master" && options?.gitMasterConfig) { - resolved.set(name, injectGitMasterConfig(template, options.gitMasterConfig)) + if (name === "git-master") { + resolved.set(name, injectGitMasterConfig(template, options?.gitMasterConfig)) } else { resolved.set(name, template) } diff --git a/src/index.ts b/src/index.ts index 9f4871efc..fd152f0a1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -281,6 +281,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { skills: mergedSkills, mcpManager: skillMcpManager, getSessionID: getSessionIDForMcp, + gitMasterConfig: pluginConfig.git_master, }); const skillMcpTool = createSkillMcpTool({ manager: skillMcpManager, diff --git a/src/plugin-handlers/config-handler.ts b/src/plugin-handlers/config-handler.ts index 55c4f24e5..27fc1458b 100644 --- a/src/plugin-handlers/config-handler.ts +++ b/src/plugin-handlers/config-handler.ts @@ -104,7 +104,8 @@ export function createConfigHandler(deps: ConfigHandlerDeps) { pluginConfig.agents, ctx.directory, config.model as string | undefined, - pluginConfig.categories + pluginConfig.categories, + pluginConfig.git_master ); // Claude Code agents: Do NOT apply permission migration diff --git a/src/tools/skill/tools.ts b/src/tools/skill/tools.ts index 9fdca0676..bc4176b90 100644 --- a/src/tools/skill/tools.ts +++ b/src/tools/skill/tools.ts @@ -4,6 +4,7 @@ import { TOOL_DESCRIPTION_NO_SKILLS, TOOL_DESCRIPTION_PREFIX } from "./constants import type { SkillArgs, SkillInfo, SkillLoadOptions } from "./types" import type { LoadedSkill } from "../../features/opencode-skill-loader" import { getAllSkills, extractSkillTemplate } from "../../features/opencode-skill-loader/skill-content" +import { injectGitMasterConfig } from "../../features/opencode-skill-loader/skill-content" import type { SkillMcpManager, SkillMcpClientInfo, SkillMcpServerContext } from "../../features/skill-mcp-manager" import type { Tool, Resource, Prompt } from "@modelcontextprotocol/sdk/types.js" @@ -164,7 +165,12 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition throw new Error(`Skill "${args.name}" not found. Available skills: ${available || "none"}`) } - const body = await extractSkillBody(skill) + let body = await extractSkillBody(skill) + + if (args.name === "git-master") { + body = injectGitMasterConfig(body, options.gitMasterConfig) + } + const dir = skill.path ? dirname(skill.path) : skill.resolvedPath || process.cwd() const output = [ diff --git a/src/tools/skill/types.ts b/src/tools/skill/types.ts index 77a8f8a75..3817158c9 100644 --- a/src/tools/skill/types.ts +++ b/src/tools/skill/types.ts @@ -1,5 +1,6 @@ import type { SkillScope, LoadedSkill } from "../../features/opencode-skill-loader/types" import type { SkillMcpManager } from "../../features/skill-mcp-manager" +import type { GitMasterConfig } from "../../config/schema" export interface SkillArgs { name: string @@ -25,4 +26,6 @@ export interface SkillLoadOptions { mcpManager?: SkillMcpManager /** Session ID getter for MCP client identification */ getSessionID?: () => string + /** Git master configuration for watermark/co-author settings */ + gitMasterConfig?: GitMasterConfig }