Browse Source

Merge pull request #902 from mhenke/omos/fix-899-resolve-prompt-warn

fix: inline prompt wins over prompt file, warn on conflict (#899)
Alvin 1 week ago
parent
commit
888cdfbbbf

+ 34 - 16
docs/project-local-customization.md

@@ -1,6 +1,6 @@
 # Project-local Customization
 
-This document describes how to configure and customize oh-my-opencode-slim on a per-project (repository-specific) basis. Project-local customization allows teams and repositories to define custom agents, override systemic prompts, restrict skills, and orchestrate MCP configurations without polluting global user configurations.
+This document describes how to configure and customize oh-my-opencode-slim on a per-project (repository-specific) basis. Project-local customization lets teams and repositories define custom agents, override systemic prompts, restrict skills, and set MCP configurations without affecting global user configurations.
 
 ## Security & Trust Boundary Warning
 
@@ -16,8 +16,8 @@ This document describes how to configure and customize oh-my-opencode-slim on a
 |---|---|---|
 | **Configuration file** | `.opencode/oh-my-opencode-slim.json[c]` | Project-level configuration file that overrides global user settings, merging presets, agent profiles, and multiplexer integration. |
 | **Custom agents** | `agents` configuration block | Define new specialized agents by keying them under `agents.<custom-name>` with required `model`, custom system `prompt`, and optional routing guidance. |
-| **Built-in prompt overrides** | `.opencode/oh-my-opencode-slim/<agent>.md` | Completely override the built-in system prompt for any agent (e.g. `oracle.md`, `explorer.md`, `orchestrator.md`, or custom agents). |
-| **Append prompts** | `.opencode/oh-my-opencode-slim/<agent>_append.md` | Append additional rules or guidelines to the existing base (inline or default built-in) prompt without overriding it completely. |
+| **Built-in prompt overrides** | `.opencode/oh-my-opencode-slim/<agent>.md` | Override the built-in system prompt for any agent (e.g. `oracle.md`, `explorer.md`, `orchestrator.md`, or custom agents). Acts as the default when no inline `prompt` is set in config. |
+| **Append prompts** | `.opencode/oh-my-opencode-slim/<agent>_append.md` | Append additional rules or guidelines to the existing base (inline, file, or default built-in) prompt without overriding it completely. |
 | **Per-agent skills** | `agents.<agent>.skills` | Explicitly restrict or authorize specific local codebase skills/scripts that this agent is allowed to execute. |
 | **Per-agent MCPs** | `agents.<agent>.mcps` | Assign, restrict, or authorize specific Model Context Protocol (MCP) servers (like `context7` or `gh_grep`) to specific agents. |
 | **Presets** | `presets` configuration block | Bundle named agent environments. User and project preset definitions deep-merge; the active preset then merges into `agents`. |
@@ -65,24 +65,38 @@ When looking up markdown prompt template files (such as `<agent>.md` or `<agent>
 
 ## Prompt Composition Rules
 
-For any agent, the final system prompt is computed dynamically using the following formula:
+For any agent, the final system prompt is computed dynamically. Precedence
+is **inline > file > built-in default**:
 
-1. **Calculate Base Prompt:**
+1. **Resolve the effective base prompt:**
    ```
-   base = inlinePrompt ?? defaultBuiltInPrompt
+   effectiveBase = inlinePrompt ?? filePrompt ?? defaultBuiltInPrompt
    ```
-   - For built-in agents, `defaultBuiltInPrompt` is their factory template.
-   - For custom agents, `defaultBuiltInPrompt` defaults to `"You are the <name> specialist."`.
-   - `inlinePrompt` is the inline `prompt` string configured directly inside the `agents.<agent>.prompt` object.
-
-2. **Calculate Effective Base:**
+   - `inlinePrompt` is the `prompt` string set directly in
+     `agents.<agent>.prompt` (config or preset).
+   - `filePrompt` is the content of the resolved `<agent>.md` replacement
+     file (located according to the Prompt Lookup Precedence).
+   - `defaultBuiltInPrompt` is the agent's factory template (built-in
+     agents) or `"You are the <name> specialist."` (custom agents).
+
+   An explicit inline `prompt` always wins over a prompt file. The file
+   acts as a shared default — useful for the "shared prompt, per-preset
+   model" pattern where the file holds the common base and each preset
+   only overrides `model` and `variant`.
+
+2. **Conflict warning:**
+   When both an inline `prompt` and a `<agent>.md` file exist, a
+   `console.warn` is emitted at agent construction:
    ```
-   effectiveBase = filePrompt ?? base
+   [oh-my-opencode] Agent '<name>': inline prompt overrides prompt file
+   (<name>.md). Remove the inline prompt to use the file.
    ```
-   - `filePrompt` is the content of the resolved `<agent>.md` replacement file (located according to the Prompt Lookup Precedence).
+   This is informational — the inline prompt takes effect as expected. The
+   warning surfaces the conflict so you know the file is being ignored.
 
-3. **Append Append Prompt:**
-   - If an append file `<agent>_append.md` is resolved, it is appended to the `effectiveBase` separated by two newlines:
+3. **Append prompt:**
+   - If an append file `<agent>_append.md` is resolved, it is appended to
+     the `effectiveBase` separated by two newlines:
      ```
      finalPrompt = effectiveBase + "\n\n" + appendPrompt
      ```
@@ -128,4 +142,8 @@ If you also place a file under `.opencode/oh-my-opencode-slim/backend-preset/ora
 ```
 Your primary focus is auditing backend security and performance.
 ```
-According to prompt composition rules, the markdown file prompt overrides the inline preset prompt, so the effective base prompt becomes `"Your primary focus is auditing backend security and performance."`.
+The inline preset `prompt` takes precedence over the file, so the effective
+base prompt becomes `"You are the project senior backend oracle. Focus
+strictly on NestJS."`. A `console.warn` fires noting the file is being
+overridden. To use the file prompt instead, remove the inline `prompt` from
+the preset config.

+ 7 - 2
src/agents/council.ts

@@ -63,8 +63,13 @@ export function createCouncilAgent(
   customAppendPrompt?: string,
 ): AgentDefinition {
   const prompt =
-    resolvePrompt(COUNCIL_AGENT_PROMPT, customPrompt, customAppendPrompt) +
-    COUNCIL_SYNTHESIS_REINFORCEMENT;
+    resolvePrompt(
+      'council',
+      customPrompt,
+      undefined,
+      COUNCIL_AGENT_PROMPT,
+      customAppendPrompt,
+    ) + COUNCIL_SYNTHESIS_REINFORCEMENT;
 
   return {
     name: 'council',

+ 3 - 1
src/agents/councillor.ts

@@ -58,8 +58,10 @@ export function createCouncillorAgent(
   variant?: string,
 ): AgentDefinition {
   const prompt = resolvePrompt(
-    COUNCILLOR_PROMPT,
+    'councillor',
     customPrompt,
+    undefined,
+    COUNCILLOR_PROMPT,
     customAppendPrompt,
   );
 

+ 13 - 10
src/agents/index.ts

@@ -235,7 +235,6 @@ function buildCustomAgentDefinition(
   const defaultPrompt = appendTaskRejectionInstruction(
     `You are the ${name} specialist.`,
   );
-  const basePrompt = override.prompt ?? defaultPrompt;
   const primaryModel = getPrimaryModelFromOverride(override);
 
   return {
@@ -243,7 +242,13 @@ function buildCustomAgentDefinition(
     config: {
       model: primaryModel ?? DEFAULT_MODELS.oracle,
       temperature: 0.2,
-      prompt: resolvePrompt(basePrompt, filePrompt, fileAppendPrompt),
+      prompt: resolvePrompt(
+        name,
+        override.prompt,
+        filePrompt,
+        defaultPrompt,
+        fileAppendPrompt,
+      ),
     },
   } as AgentDefinition;
 }
@@ -401,11 +406,11 @@ export function createAgents(
         agent.config.prompt ?? '',
       );
 
-      const basePrompt =
-        inlinePrompt !== undefined ? inlinePrompt : defaultPrompt;
       agent.config.prompt = resolvePrompt(
-        basePrompt,
+        name,
+        inlinePrompt,
         customPrompts.prompt,
+        defaultPrompt,
         customPrompts.appendPrompt,
       );
 
@@ -550,13 +555,11 @@ export function createAgents(
   const inlineOrchestratorPrompt = orchestratorOverride?.prompt;
   const defaultOrchestratorPrompt = orchestrator.config.prompt ?? '';
 
-  const baseOrchestratorPrompt =
-    inlineOrchestratorPrompt !== undefined
-      ? inlineOrchestratorPrompt
-      : defaultOrchestratorPrompt;
   orchestrator.config.prompt = resolvePrompt(
-    baseOrchestratorPrompt,
+    'orchestrator',
+    inlineOrchestratorPrompt,
     orchestratorPrompts.prompt,
+    defaultOrchestratorPrompt,
     orchestratorPrompts.appendPrompt,
   );
 

+ 23 - 7
src/agents/orchestrator.ts

@@ -11,16 +11,26 @@ export interface AgentDefinition {
 }
 
 /**
- * Resolve agent prompt from base/custom/append inputs.
- * If customPrompt is provided, it replaces the base entirely.
- * If customAppendPrompt is provided, it appends after whichever base won.
+ * Resolve agent prompt from inline/file/append inputs.
+ *
+ * Precedence: inline prompt > file prompt > fallback. An explicit inline
+ * `override.prompt` wins over a `<agent>.md` file; the file is the
+ * shared default. `customAppendPrompt` always appends after whichever base
+ * won. Deterministic per session (construction-time only) — cache-safe.
  */
 export function resolvePrompt(
-  base: string,
-  customPrompt?: string,
+  agentName: string,
+  inlinePrompt: string | undefined,
+  filePrompt: string | undefined,
+  fallback: string,
   customAppendPrompt?: string,
 ): string {
-  const effectiveBase = customPrompt !== undefined ? customPrompt : base;
+  if (inlinePrompt !== undefined && filePrompt !== undefined) {
+    console.warn(
+      `[oh-my-opencode] Agent '${agentName}': inline prompt overrides prompt file (${agentName}.md). Remove the inline prompt to use the file.`,
+    );
+  }
+  const effectiveBase = inlinePrompt ?? filePrompt ?? fallback;
   return customAppendPrompt !== undefined
     ? `${effectiveBase}\n\n${customAppendPrompt}`
     : effectiveBase;
@@ -298,7 +308,13 @@ export function createOrchestratorAgent(
     excludeDescriptions,
     waitForUserEnabled,
   );
-  const prompt = resolvePrompt(basePrompt, customPrompt, customAppendPrompt);
+  const prompt = resolvePrompt(
+    'orchestrator',
+    undefined,
+    customPrompt,
+    basePrompt,
+    customAppendPrompt,
+  );
 
   const definition: AgentDefinition = {
     name: 'orchestrator',

+ 67 - 0
src/agents/resolve-prompt-warn.test.ts

@@ -0,0 +1,67 @@
+import { afterEach, describe, expect, spyOn, test } from 'bun:test';
+import { resolvePrompt } from './orchestrator';
+
+const FALLBACK = 'fallback prompt';
+const FILE = 'file prompt';
+const INLINE = 'inline prompt';
+const APPEND = 'append prompt';
+
+afterEach(() => {
+  spyOn(console, 'warn').mockRestore();
+});
+
+describe('resolvePrompt precedence', () => {
+  test('inline wins over file and fallback', () => {
+    expect(resolvePrompt('a', INLINE, FILE, FALLBACK)).toBe(INLINE);
+  });
+
+  test('file wins over fallback when no inline', () => {
+    expect(resolvePrompt('a', undefined, FILE, FALLBACK)).toBe(FILE);
+  });
+
+  test('fallback used when no inline and no file', () => {
+    expect(resolvePrompt('a', undefined, undefined, FALLBACK)).toBe(FALLBACK);
+  });
+
+  test('append concatenated after whichever base won', () => {
+    expect(resolvePrompt('a', INLINE, FILE, FALLBACK, APPEND)).toBe(
+      `${INLINE}\n\n${APPEND}`,
+    );
+    expect(resolvePrompt('a', undefined, FILE, FALLBACK, APPEND)).toBe(
+      `${FILE}\n\n${APPEND}`,
+    );
+    expect(resolvePrompt('a', undefined, undefined, FALLBACK, APPEND)).toBe(
+      `${FALLBACK}\n\n${APPEND}`,
+    );
+  });
+});
+
+describe('resolvePrompt conflict warning', () => {
+  test('warns when both inline and file prompt present', () => {
+    const warn = spyOn(console, 'warn').mockImplementation(() => {});
+    resolvePrompt('skeptic', INLINE, FILE, FALLBACK);
+    expect(warn).toHaveBeenCalledTimes(1);
+    const msg = warn.mock.calls[0][0] as string;
+    expect(msg).toContain("'skeptic'");
+    expect(msg).toContain('skeptic.md');
+    expect(msg).toContain('overrides');
+  });
+
+  test('does not warn when only inline prompt present', () => {
+    const warn = spyOn(console, 'warn').mockImplementation(() => {});
+    resolvePrompt('skeptic', INLINE, undefined, FALLBACK);
+    expect(warn).not.toHaveBeenCalled();
+  });
+
+  test('does not warn when only file prompt present', () => {
+    const warn = spyOn(console, 'warn').mockImplementation(() => {});
+    resolvePrompt('skeptic', undefined, FILE, FALLBACK);
+    expect(warn).not.toHaveBeenCalled();
+  });
+
+  test('does not warn when neither present', () => {
+    const warn = spyOn(console, 'warn').mockImplementation(() => {});
+    resolvePrompt('skeptic', undefined, undefined, FALLBACK);
+    expect(warn).not.toHaveBeenCalled();
+  });
+});

+ 5 - 3
src/config/project-local-customization.test.ts

@@ -129,8 +129,8 @@ describe('Project-local customization - 15 core cases', () => {
     );
   });
 
-  // Test Case 6: File prompt overrides inline built-in prompt
-  test('6. File prompt overrides inline built-in prompt', () => {
+  // Test Case 6: Inline prompt overrides file prompt
+  test('6. Inline prompt overrides file prompt', () => {
     const config = {
       agents: {
         oracle: {
@@ -150,7 +150,9 @@ describe('Project-local customization - 15 core cases', () => {
 
     const agents = createAgents(config);
     const oracle = agents.find((a) => a.name === 'oracle');
-    expect(oracle?.config.prompt).toBe('File prompt override content');
+    expect(oracle?.config.prompt).toBe(
+      'You are the inline oracle prompt override.',
+    );
   });
 
   // Test Case 7: Append file appends to inline built-in prompt