Browse Source

fix(hooks): make file-tool nudge ephemeral (#269)

* Prevent file-tool nudges from polluting context

The file-tool nudge was written into persisted Read/Write outputs, so repeated file operations could replay workflow guidance as if it were tool data. This keeps the delegation reminder but moves it into a one-shot system prompt path keyed by session.

Constraint: Preserve the original delegation reminder semantics

Constraint: Do not modify OpenCode core or add a new hook module

Rejected: Remove the nudge entirely | loses the delegation guardrail

Rejected: Keep appending to tool output with marker tags | still pollutes persisted tool data

Confidence: high

Scope-risk: narrow

Directive: Keep workflow guidance out of persisted tool outputs; use ephemeral prompt surfaces for nudges

Tested: bun test src/hooks/post-file-tool-nudge/index.test.ts

Tested: bun run check:ci

Tested: bun run typecheck

Tested: bun run build

Tested: bun test

* Update src/hooks/post-file-tool-nudge/index.ts

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

---------

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Raxxoor 3 months ago
parent
commit
989a0e0a29

+ 2 - 2
codemap.md

@@ -49,7 +49,7 @@ The plugin integrates with OpenCode to provide:
 | `src/hooks/` | Lifecycle hooks for message transforms, error recovery, and rate-limit fallbacks. | [View Map](src/hooks/codemap.md) |
 | `src/hooks/auto-update-checker/` | Startup update check hook with cache invalidation and optional auto-install. | [View Map](src/hooks/auto-update-checker/codemap.md) |
 | `src/hooks/phase-reminder/` | Orchestrator message transform hook that injects phase reminders. | [View Map](src/hooks/phase-reminder/codemap.md) |
-| `src/hooks/post-read-nudge/` | Read tool after-hook that appends delegation nudges. | [View Map](src/hooks/post-read-nudge/codemap.md) |
+| `src/hooks/post-file-tool-nudge/` | Read/Write tool after-hook that queues ephemeral delegation nudges. | [View Map](src/hooks/post-file-tool-nudge/codemap.md) |
 | `src/hooks/delegate-task-retry/` | Error detection and retry guidance with pattern matching and assistance. | [View Map](src/hooks/delegate-task-retry/codemap.md) |
 | `src/hooks/foreground-fallback/` | Rate-limit fallback manager for interactive sessions. | [View Map](src/hooks/foreground-fallback/codemap.md) |
 | `src/hooks/json-error-recovery/` | JSON parse error detection and recovery helpers. | [View Map](src/hooks/json-error-recovery/codemap.md) |
@@ -206,4 +206,4 @@ interface PluginConfig {
 
 ## License
 
-MIT License - See [LICENSE](LICENSE) for details.
+MIT License - See [LICENSE](LICENSE) for details.

+ 2 - 2
src/codemap.md

@@ -8,7 +8,7 @@
 ## Design
 - Agent creation follows explicit factories (`agents/index.ts`, per-agent creators under `agents/`) with override/permission helpers (`config/utils.ts`, `cli/skills.ts`) so defaults live in `config/constants.ts`, prompts can be swapped via `config/loader.ts`, and variant labels propagate through `utils/agent-variant.ts`.
 - Background tooling composes `BackgroundTaskManager`, `TmuxSessionManager`, and `createBackgroundTools` (which uses `tool` with Zod schemas) to provide async/sync task launches plus cancel/output helpers; polling/prompt flow lives in `tools/background.ts` while TMUX lifecycle uses `utils/tmux.ts` to spawn/close panes and reapply layouts.
-- Hooks are isolated (`hooks/auto-update-checker`, `phase-reminder`, `post-read-nudge`) and exported via `hooks/index.ts`, so the plugin simply registers them via the `event`, `experimental.chat.messages.transform`, and `tool.execute.after` hooks defined in `index.ts`.
+- Hooks are isolated (`hooks/auto-update-checker`, `phase-reminder`, `post-file-tool-nudge`) and exported via `hooks/index.ts`, so the plugin simply registers them via the `event`, `experimental.chat.system.transform`, `experimental.chat.messages.transform`, and `tool.execute.after` hooks defined in `index.ts`.
 - Supplemental tools (`tools/grep`, `tools/lsp`, `tools/quota`) bundle ripgrep, LSP helpers, and Antigravity quota calls behind the OpenCode `tool` interface and are mounted in `index.ts` alongside background/task tools.
 
 ## Flow
@@ -20,5 +20,5 @@
 ## Integration
 - Connects directly to the OpenCode plugin API (`@opencode-ai/plugin`): registers agents/tools/mcps, responds to `session.created` and `tool.execute.after` events, injects `experimental.chat.messages.transform`, and makes RPC calls via `ctx.client`/`ctx.client.session` throughout `tools/background` and `background/*`.
 - Integrates with the host environment: `utils/tmux.ts` checks for tmux and server availability, `startTmuxCheck` pre-seeds the binary path, and `TmuxSessionManager`/`BackgroundTaskManager` coordinate via shared configuration and `tools/background` to keep CLI panes synchronized.
-- Hooks and helpers tie into external behavior: `hooks/auto-update-checker` reads `package.json` metadata, runs safe `bun install`, and posts toasts; `hooks/phase-reminder/post-read-nudge` enforce workflow reminders; `utils/logger.ts` centralizes structured logging used across modules.
+- Hooks and helpers tie into external behavior: `hooks/auto-update-checker` reads `package.json` metadata, runs safe `bun install`, and posts toasts; `hooks/phase-reminder/post-file-tool-nudge` enforce workflow reminders without mutating file tool output; `utils/logger.ts` centralizes structured logging used across modules.
 - CLI utilities modify OpenCode CLI/user config files (`cli/config-manager.ts`) and install additional skills/ providers, ensuring the plugin lands with the expected agents, provider auth helpers, and custom skill definitions.

+ 8 - 7
src/hooks/codemap.md

@@ -27,10 +27,11 @@ Acts as a single entry point that re-exports the factory functions and types for
 
 | Hook Point | Purpose | Hooks |
 |------------|---------|-------|
-| `'tool.execute.after'` | Modify tool output after execution | `post-read-nudge`, `delegate-task-retry`, `json-error-recovery` |
+| `'tool.execute.after'` | React to tool output after execution | `post-file-tool-nudge`, `delegate-task-retry`, `json-error-recovery` |
+| `'experimental.chat.system.transform'` | Transform system prompts before API call | `post-file-tool-nudge` |
 | `'experimental.chat.messages.transform'` | Transform messages before API call | `phase-reminder` |
 | `'chat.headers'` | Add custom headers to API requests | `chat-headers` |
-| Event handlers | React to OpenCode events | `foreground-fallback` |
+| Event handlers | React to OpenCode events | `foreground-fallback`, `post-file-tool-nudge` |
 
 ### Hook Implementations
 
@@ -41,11 +42,11 @@ Acts as a single entry point that re-exports the factory functions and types for
 - **Behavior**: Prepend reminder text to the last user message if agent is 'orchestrator' and message doesn't contain internal initiator marker.
 - **Research**: Based on "LLMs Get Lost In Multi-Turn Conversation" (arXiv:2505.06120) showing ~40% compliance drop after 2-3 turns without reminders.
 
-#### **post-read-nudge**
-- **Location**: `src/hooks/post-read-nudge/index.ts`
-- **Purpose**: Appends delegation reminder after file reads to catch the "read files → implement myself" anti-pattern.
-- **Hook Point**: `'tool.execute.after'`
-- **Behavior**: Appends nudge text to output when tool is 'Read' or 'read'.
+#### **post-file-tool-nudge**
+- **Location**: `src/hooks/post-file-tool-nudge/index.ts`
+- **Purpose**: Queues a delegation reminder after file reads/writes to catch the "inspect/edit files → implement myself" anti-pattern.
+- **Hook Points**: `'tool.execute.after'`, `'experimental.chat.system.transform'`
+- **Behavior**: Records a pending reminder when Read/Write tools run, consumes it once in the next system prompt transform without mutating persisted tool output, and clears stale pending markers on session deletion.
 
 #### **chat-headers**
 - **Location**: `src/hooks/chat-headers.ts`

+ 4 - 4
src/hooks/post-file-tool-nudge/codemap.md

@@ -4,16 +4,16 @@
 
 ## Responsibility
 
-Provide a lightweight safety net that reminds agents to stay in the delegation workflow whenever they read or write project files. When the Read or Write tool emits output, this hook appends a standardized nudge so that follow-up work is routed to the right specialists instead of being solved immediately.
+Provide a lightweight safety net that reminds the orchestrator to stay in the delegation workflow whenever it reads or writes project files. Read/Write tool results are treated as evidence only; the hook now queues a one-shot reminder for the next system prompt instead of modifying the persisted tool output.
 
 ## Design
 
-Exports a single factory (`createPostFileToolNudgeHook`) that returns a handler map for the `tool.execute.after` event. The hook keeps a shared `NUDGE` constant and enforces a policy guard for Read/Write tool names before mutating the `output`. This keeps the implementation focused and pluggable inside the broader hook registry.
+Exports a single factory (`createPostFileToolNudgeHook`) that returns handlers for `tool.execute.after`, `experimental.chat.system.transform`, and session lifecycle events. The tool hook records pending session IDs for Read/Write tools, while the system transform consumes each pending ID once and appends the existing workflow reminder only when the caller allows injection for that session. The event handler clears stale pending IDs when sessions are deleted. This preserves the delegation behavior without contaminating persisted tool output.
 
 ## Flow
 
-The hook is instantiated once and registered with the hook system, which invokes `tool.execute.after` after every tool call. When a Read or Write tool completes, the hook sees the input metadata, verifies the tool name, and appends the reminder text to `output.output`, thereby altering the user-facing response before it bubbles back to the agent.
+The hook is instantiated once and registered with the hook system. When a Read or Write tool completes, `tool.execute.after` verifies the tool name and session ID, then stores the session ID in an in-memory pending set. On the next `experimental.chat.system.transform` call for that session, the hook deletes the pending marker and appends the workflow reminder to the outgoing system prompt. Multiple Read/Write calls before the next model turn collapse into one reminder, non-orchestrator sessions can be consumed without injection, and deleted sessions are removed from the pending set.
 
 ## Integration
 
-Plugged into the global hook registry, this module intercepts every tool response via the `tool.execute.after` lifecycle event. It directly touches only Read/Write tool output objects so there are no downstream dependencies, but the appended reminder influences all downstream consumers who display file-tool results.
+Plugged into the global hook registry, this module intercepts every tool response via the `tool.execute.after` lifecycle event, participates in system prompt transformation via `experimental.chat.system.transform`, and listens to session deletion events for cleanup. It intentionally does not mutate `output.output`, so file contents remain clean for persistence, replay, and compaction while the orchestrator still receives the intended delegation reminder.

+ 185 - 0
src/hooks/post-file-tool-nudge/index.test.ts

@@ -0,0 +1,185 @@
+import { describe, expect, test } from 'bun:test';
+
+import { PHASE_REMINDER_TEXT } from '../../config/constants';
+import { createPostFileToolNudgeHook } from './index';
+
+function createOutput(output = 'real content') {
+  return {
+    title: 'Read',
+    output,
+    metadata: {},
+  };
+}
+
+function countReminder(system: string[]) {
+  return system.join('\n').split(PHASE_REMINDER_TEXT).length - 1;
+}
+
+describe('post-file-tool-nudge hook', () => {
+  test('does not contaminate persisted Read output', async () => {
+    const hook = createPostFileToolNudgeHook();
+    const output = createOutput();
+
+    await hook['tool.execute.after']({ tool: 'read', sessionID: 's1' }, output);
+
+    expect(output.output).toBe('real content');
+    expect(output.output).not.toContain(PHASE_REMINDER_TEXT);
+  });
+
+  test('injects the delegation reminder in system transform', async () => {
+    const hook = createPostFileToolNudgeHook();
+    const output = createOutput();
+    const system = { system: ['base system prompt'] };
+
+    await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, output);
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      system,
+    );
+
+    expect(output.output).toBe('real content');
+    expect(system.system.join('\n')).toContain(PHASE_REMINDER_TEXT);
+  });
+
+  test('consumes the reminder once', async () => {
+    const hook = createPostFileToolNudgeHook();
+    const first = { system: ['base'] };
+    const second = { system: ['base'] };
+
+    await hook['tool.execute.after'](
+      { tool: 'write', sessionID: 's1' },
+      createOutput(),
+    );
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      first,
+    );
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      second,
+    );
+
+    expect(countReminder(first.system)).toBe(1);
+    expect(countReminder(second.system)).toBe(0);
+  });
+
+  test('deduplicates multiple Read/Write calls before the next prompt', async () => {
+    const hook = createPostFileToolNudgeHook();
+    const system = { system: ['base'] };
+
+    await hook['tool.execute.after'](
+      { tool: 'read', sessionID: 's1' },
+      createOutput(),
+    );
+    await hook['tool.execute.after'](
+      { tool: 'write', sessionID: 's1' },
+      createOutput(),
+    );
+    await hook['tool.execute.after'](
+      { tool: 'Read', sessionID: 's1' },
+      createOutput(),
+    );
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      system,
+    );
+
+    expect(countReminder(system.system)).toBe(1);
+  });
+
+  test('ignores non-file tools', async () => {
+    const hook = createPostFileToolNudgeHook();
+    const output = createOutput('ok');
+    const system = { system: ['base'] };
+
+    await hook['tool.execute.after']({ tool: 'bash', sessionID: 's1' }, output);
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      system,
+    );
+
+    expect(output.output).toBe('ok');
+    expect(countReminder(system.system)).toBe(0);
+  });
+
+  test('consumes without injecting when the session should not receive the nudge', async () => {
+    const hook = createPostFileToolNudgeHook({ shouldInject: () => false });
+    const first = { system: ['base'] };
+    const second = { system: ['base'] };
+
+    await hook['tool.execute.after'](
+      { tool: 'read', sessionID: 's1' },
+      createOutput(),
+    );
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      first,
+    );
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      second,
+    );
+
+    expect(countReminder(first.system)).toBe(0);
+    expect(countReminder(second.system)).toBe(0);
+  });
+
+  test('ignores Read/Write without sessionID', async () => {
+    const hook = createPostFileToolNudgeHook();
+    const output = createOutput();
+    const system = { system: ['base'] };
+
+    await hook['tool.execute.after']({ tool: 'read' }, output);
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      system,
+    );
+
+    expect(output.output).toBe('real content');
+    expect(countReminder(system.system)).toBe(0);
+  });
+
+  test('cleans up pending reminders when a session is deleted', async () => {
+    const hook = createPostFileToolNudgeHook();
+    const system = { system: ['base'] };
+
+    await hook['tool.execute.after'](
+      { tool: 'read', sessionID: 's1' },
+      createOutput(),
+    );
+    await hook.event({
+      event: {
+        type: 'session.deleted',
+        properties: { info: { id: 's1' } },
+      },
+    });
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      system,
+    );
+
+    expect(countReminder(system.system)).toBe(0);
+  });
+
+  test('cleans up pending reminders from sessionID delete events', async () => {
+    const hook = createPostFileToolNudgeHook();
+    const system = { system: ['base'] };
+
+    await hook['tool.execute.after'](
+      { tool: 'write', sessionID: 's1' },
+      createOutput(),
+    );
+    await hook.event({
+      event: {
+        type: 'session.deleted',
+        properties: { sessionID: 's1' },
+      },
+    });
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      system,
+    );
+
+    expect(countReminder(system.system)).toBe(0);
+  });
+});

+ 60 - 17
src/hooks/post-file-tool-nudge/index.ts

@@ -1,11 +1,11 @@
 /**
- * Post-tool nudge - appends a delegation reminder after file reads/writes.
+ * Post-tool nudge - queues a delegation reminder after file reads/writes.
  * Catches the "inspect/edit files → implement myself" anti-pattern.
  */
 
 import { PHASE_REMINDER_TEXT } from '../../config/constants';
 
-const NUDGE = `\n\n---\n${PHASE_REMINDER_TEXT}`;
+const POST_FILE_TOOL_NUDGE = PHASE_REMINDER_TEXT;
 
 interface ToolExecuteAfterInput {
   tool: string;
@@ -13,30 +13,73 @@ interface ToolExecuteAfterInput {
   callID?: string;
 }
 
-interface ToolExecuteAfterOutput {
-  title: string;
-  output: string;
-  metadata: Record<string, unknown>;
+interface ChatSystemTransformInput {
+  sessionID?: string;
+}
+
+interface ChatSystemTransformOutput {
+  system: string[];
 }
 
-export function createPostFileToolNudgeHook() {
+interface EventInput {
+  event: {
+    type: string;
+    properties?: {
+      info?: { id?: string };
+      sessionID?: string;
+    };
+  };
+}
+
+interface PostFileToolNudgeOptions {
+  shouldInject?: (sessionID: string) => boolean;
+}
+
+const FILE_TOOLS = new Set(['Read', 'read', 'Write', 'write']);
+
+export function createPostFileToolNudgeHook(
+  options: PostFileToolNudgeOptions = {},
+) {
+  const pendingSessionIds = new Set<string>();
+
   return {
     'tool.execute.after': async (
       input: ToolExecuteAfterInput,
-      output: ToolExecuteAfterOutput,
+      _output: unknown,
     ): Promise<void> => {
-      // Only nudge for Read/Write tools
-      if (
-        input.tool !== 'Read' &&
-        input.tool !== 'read' &&
-        input.tool !== 'Write' &&
-        input.tool !== 'write'
-      ) {
+      // Only nudge for Read/Write tools once the next model call is built.
+      if (!FILE_TOOLS.has(input.tool) || !input.sessionID) {
+        return;
+      }
+
+      pendingSessionIds.add(input.sessionID);
+    },
+    'experimental.chat.system.transform': async (
+      input: ChatSystemTransformInput,
+      output: ChatSystemTransformOutput,
+    ): Promise<void> => {
+      if (!input.sessionID || !pendingSessionIds.delete(input.sessionID)) {
+        return;
+      }
+
+      if (options.shouldInject && !options.shouldInject(input.sessionID)) {
+        return;
+      }
+
+      output.system.push(POST_FILE_TOOL_NUDGE);
+    },
+    event: async (input: EventInput): Promise<void> => {
+      if (input.event.type !== 'session.deleted') {
+        return;
+      }
+
+      const sessionID =
+        input.event.properties?.sessionID ?? input.event.properties?.info?.id;
+      if (!sessionID) {
         return;
       }
 
-      // Append the nudge
-      output.output = output.output + NUDGE;
+      pendingSessionIds.delete(sessionID);
     },
   };
 }

+ 24 - 5
src/index.ts

@@ -147,8 +147,14 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
     config,
   );
 
+  // Track session → agent mapping for serve-mode system prompt injection
+  const sessionAgentMap = new Map<string, string>();
+
   // Initialize post-file-tool nudge hook
-  const postFileToolNudgeHook = createPostFileToolNudgeHook();
+  const postFileToolNudgeHook = createPostFileToolNudgeHook({
+    shouldInject: (sessionID) =>
+      sessionAgentMap.get(sessionID) === 'orchestrator',
+  });
 
   const chatHeadersHook = createChatHeadersHook(ctx);
 
@@ -165,9 +171,6 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
     config.fallback?.enabled !== false && Object.keys(runtimeChains).length > 0,
   );
 
-  // Track session → agent mapping for serve-mode system prompt injection
-  const sessionAgentMap = new Map<string, string>();
-
   // Initialize todo-continuation hook (opt-in auto-continue for incomplete todos)
   const todoContinuationHook = createTodoContinuationHook(ctx, {
     maxContinuations: config.todoContinuation?.maxContinuations ?? 5,
@@ -447,6 +450,18 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
           event: { type: string; properties?: Record<string, unknown> };
         },
       );
+
+      await postFileToolNudgeHook.event(
+        input as {
+          event: {
+            type: string;
+            properties?: {
+              info?: { id?: string };
+              sessionID?: string;
+            };
+          };
+        },
+      );
     },
 
     // Direct interception of /auto-continue command — bypasses LLM round-trip
@@ -501,9 +516,13 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
           const { ORCHESTRATOR_PROMPT } = await import('./agents/orchestrator');
           output.system[0] =
             ORCHESTRATOR_PROMPT +
-            (output.system[0] ? '\n\n' + output.system[0] : '');
+            (output.system[0] ? `\n\n${output.system[0]}` : '');
         }
       }
+      await postFileToolNudgeHook['experimental.chat.system.transform'](
+        input,
+        output,
+      );
     },
 
     // Inject phase reminder and filter available skills before sending to API (doesn't show in UI)