Merge pull request #402 from dhaern/fix/plugin-side-performance-overhead

fix: improve plugin-side performance and reduce runtime overhead

src/council/council-manager.ts

@@ -350,8 +350,9 @@ export class CouncilManager {
       // Parallel execution (default): run all councillors concurrently
       const promises = entries.map(([name, config], index) =>
         (async () => {
-          // Stagger launches to avoid tmux split-window collisions
-          if (index > 0) {
+          // Stagger launches only when multiplexer panes can be created.
+          // Outside tmux/zellij this delay only adds latency with no benefit.
+          if (this.tmuxEnabled && index > 0) {
             await new Promise((r) =>
               setTimeout(r, index * COUNCILLOR_STAGGER_MS),
             );

src/hooks/chat-headers.test.ts

@@ -23,13 +23,15 @@ function createMockContext(parts: unknown[] = []) {
 
 function createInput(
   overrides?: Partial<{
+    sessionID: string;
     providerID: string;
     npm: string;
     messageID: string;
   }>,
 ) {
+  const sessionID = overrides?.sessionID ?? 'session-1';
   return {
-    sessionID: 'session-1',
+    sessionID,
     agent: 'orchestrator',
     model: {
       id: 'github-copilot/claude',
@@ -80,7 +82,7 @@ function createInput(
     },
     message: {
       id: overrides?.messageID ?? 'message-1',
-      sessionID: 'session-1',
+      sessionID,
       role: 'user' as const,
       time: { created: Date.now() },
       agent: 'orchestrator',
@@ -152,4 +154,83 @@ describe('createChatHeadersHook', () => {
 
     expect(output.headers['x-initiator']).toBeUndefined();
   });
+
+  test('caches marked internal messages', async () => {
+    const ctx = createMockContext([
+      createInternalAgentTextPart('internal notification'),
+    ]);
+    const hook = createChatHeadersHook(ctx);
+    const firstOutput = { headers: {} };
+    const secondOutput = { headers: {} };
+
+    await hook['chat.headers'](
+      createInput({ messageID: 'message-internal' }),
+      firstOutput,
+    );
+    await hook['chat.headers'](
+      createInput({ messageID: 'message-internal' }),
+      secondOutput,
+    );
+
+    expect(firstOutput.headers['x-initiator']).toBe('agent');
+    expect(secondOutput.headers['x-initiator']).toBe('agent');
+    expect(ctx.client.session.message).toHaveBeenCalledTimes(1);
+  });
+
+  test('does not cache transient message lookup failures', async () => {
+    let calls = 0;
+    const messageMock = mock(async () => {
+      calls += 1;
+      if (calls === 1) {
+        throw new Error('temporary failure');
+      }
+      return {
+        data: {
+          info: { role: 'user' },
+          parts: [createInternalAgentTextPart('internal notification')],
+        },
+      };
+    });
+    const ctx = {
+      client: {
+        session: {
+          message: messageMock,
+        },
+      },
+    } as unknown as PluginInput;
+    const hook = createChatHeadersHook(ctx);
+    const firstOutput = { headers: {} };
+    const secondOutput = { headers: {} };
+
+    await hook['chat.headers'](
+      createInput({ messageID: 'message-retry' }),
+      firstOutput,
+    );
+    await hook['chat.headers'](
+      createInput({ messageID: 'message-retry' }),
+      secondOutput,
+    );
+
+    expect(firstOutput.headers['x-initiator']).toBeUndefined();
+    expect(secondOutput.headers['x-initiator']).toBe('agent');
+    expect(messageMock).toHaveBeenCalledTimes(2);
+  });
+
+  test('caches marked messages by session and message id', async () => {
+    const ctx = createMockContext([
+      createInternalAgentTextPart('internal notification'),
+    ]);
+    const hook = createChatHeadersHook(ctx);
+
+    await hook['chat.headers'](
+      createInput({ sessionID: 'session-a', messageID: 'message-normal' }),
+      { headers: {} },
+    );
+    await hook['chat.headers'](
+      createInput({ sessionID: 'session-b', messageID: 'message-normal' }),
+      { headers: {} },
+    );
+
+    expect(ctx.client.session.message).toHaveBeenCalledTimes(2);
+  });
 });

src/hooks/filter-available-skills/index.test.ts

@@ -231,4 +231,66 @@ describe('createFilterAvailableSkillsHook', () => {
     );
     expect(output.messages[1].parts[0].text).toContain('No skills available.');
   });
+
+  test('reuses permission rules without caching the final skills block text', async () => {
+    const config: PluginConfig = {
+      agents: {
+        explorer: {
+          skills: ['skill1', 'skill3'],
+        },
+      },
+    };
+
+    const hook = createFilterAvailableSkillsHook(mockCtx, config);
+    const firstOutput = {
+      messages: [
+        {
+          info: { role: 'system' },
+          parts: [
+            {
+              type: 'text',
+              text: availableSkillsBlock('skill1', 'skill2'),
+            },
+          ],
+        },
+        {
+          info: { role: 'user', agent: 'explorer' },
+          parts: [{ type: 'text', text: 'check skills' }],
+        },
+      ],
+    };
+    const secondOutput = {
+      messages: [
+        {
+          info: { role: 'system' },
+          parts: [
+            {
+              type: 'text',
+              text: availableSkillsBlock('skill2', 'skill3'),
+            },
+          ],
+        },
+        {
+          info: { role: 'user', agent: 'explorer' },
+          parts: [{ type: 'text', text: 'check skills' }],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, firstOutput);
+    await hook['experimental.chat.messages.transform']({}, secondOutput);
+
+    expect(firstOutput.messages[0].parts[0].text).toContain(
+      '<name>skill1</name>',
+    );
+    expect(firstOutput.messages[0].parts[0].text).not.toContain(
+      '<name>skill3</name>',
+    );
+    expect(secondOutput.messages[0].parts[0].text).not.toContain(
+      '<name>skill1</name>',
+    );
+    expect(secondOutput.messages[0].parts[0].text).toContain(
+      '<name>skill3</name>',
+    );
+  });
 });

src/hooks/filter-available-skills/index.ts

@@ -107,6 +107,23 @@ export function createFilterAvailableSkillsHook(
   _ctx: PluginInput,
   config: PluginConfig,
 ) {
+  const permissionRulesByAgent = new Map<string, Record<string, SkillRule>>();
+
+  const getPermissionRules = (agentName: string): Record<string, SkillRule> => {
+    const cached = permissionRulesByAgent.get(agentName);
+    if (cached) {
+      return cached;
+    }
+
+    const configuredSkills = getAgentOverride(config, agentName)?.skills;
+    const permissionRules = getSkillPermissionsForAgent(
+      agentName,
+      configuredSkills,
+    );
+    permissionRulesByAgent.set(agentName, permissionRules);
+    return permissionRules;
+  };
+
   return {
     'experimental.chat.messages.transform': async (
       _input: Record<string, never>,
@@ -118,11 +135,7 @@ export function createFilterAvailableSkillsHook(
       }
 
       const agentName = getCurrentAgent(messages);
-      const configuredSkills = getAgentOverride(config, agentName)?.skills;
-      const permissionRules = getSkillPermissionsForAgent(
-        agentName,
-        configuredSkills,
-      );
+      const permissionRules = getPermissionRules(agentName);
 
       for (const message of messages) {
         for (const part of message.parts) {

src/hooks/image-hook.ts

@@ -172,9 +172,28 @@ export function processImageAttachments(args: {
   const observerEnabled = !disabledAgents.has('observer');
   if (!observerEnabled) return;
 
+  const messagesWithImages: Array<{
+    msg: MessageWithParts;
+    imageParts: ImagePart[];
+  }> = [];
+
+  for (const msg of messages) {
+    if (msg.info.role !== 'user') continue;
+    const imageParts = msg.parts.filter(isImagePart);
+    if (imageParts.length > 0) {
+      messagesWithImages.push({ msg, imageParts });
+    }
+  }
+
   // Save images inside the project's .opencode/images/ directory.
   // This is within the workspace so the read tool won't require extra permissions.
   const saveDir = join(workDir, '.opencode', 'images');
+
+  if (messagesWithImages.length === 0) {
+    if (existsSync(saveDir)) cleanupAllSessions(saveDir);
+    return;
+  }
+
   const gitignorePath = join(workDir, '.opencode', '.gitignore');
   try {
     mkdirSync(saveDir, { recursive: true });
@@ -185,11 +204,7 @@ export function processImageAttachments(args: {
 
   cleanupAllSessions(saveDir);
 
-  for (const msg of messages) {
-    if (msg.info.role !== 'user') continue;
-    const imageParts = msg.parts.filter(isImagePart);
-    if (imageParts.length === 0) continue;
-
+  for (const { msg, imageParts } of messagesWithImages) {
     const sessionSubdir = msg.info.sessionID
       ? sanitizeFilename(msg.info.sessionID)
       : undefined;

src/index.ts

@@ -32,7 +32,10 @@ import {
   createPresetManager,
   createWebfetchTool,
 } from './tools';
-import { resolveRuntimeAgentName, rewriteDisplayNameMentions } from './utils';
+import {
+  createDisplayNameMentionRewriter,
+  resolveRuntimeAgentName,
+} from './utils';
 import { initLogger, log } from './utils/logger';
 import { SubagentDepthTracker } from './utils/subagent-depth';
 import { collapseSystemInPlace } from './utils/system-collapse';
@@ -89,6 +92,7 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
   // Declare variables that must survive the try/catch for the return
   // closure. These are set inside the try block.
   let config: ReturnType<typeof loadPluginConfig>;
+  let disabledAgents: Set<string>;
   let agentDefs: ReturnType<typeof createAgents>;
   let agents: ReturnType<typeof getAgentConfigs>;
   let mcps: ReturnType<typeof createBuiltinMcps>;
@@ -116,12 +120,17 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
   let presetManager: ReturnType<typeof createPresetManager>;
   let councilTools: Record<string, unknown>;
   let webfetch: ReturnType<typeof createWebfetchTool>;
+  let rewriteDisplayNameMentions: ReturnType<
+    typeof createDisplayNameMentionRewriter
+  >;
 
   // Counters for post-init health check (set inside try, checked outside)
   let toolCount = 0;
 
   try {
     config = loadPluginConfig(ctx.directory);
+    disabledAgents = getDisabledAgents(config);
+    rewriteDisplayNameMentions = createDisplayNameMentionRewriter(config);
     agentDefs = createAgents(config);
     agents = getAgentConfigs(config);
 
@@ -174,7 +183,9 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
     // Get multiplexer instance for capability checks
     const multiplexer = getMultiplexer(multiplexerConfig);
     multiplexerEnabled =
-      multiplexerConfig.type !== 'none' && multiplexer !== null;
+      multiplexerConfig.type !== 'none' &&
+      multiplexer !== null &&
+      multiplexer.isInsideSession();
 
     log('[plugin] initialized with multiplexer config', {
       multiplexerConfig,
@@ -729,7 +740,7 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
           const orchestratorPrompt =
             typeof orchestratorDef?.config?.prompt === 'string'
               ? orchestratorDef.config.prompt
-              : buildOrchestratorPrompt(getDisabledAgents(config));
+              : buildOrchestratorPrompt(disabledAgents);
           output.system[0] =
             orchestratorPrompt +
             (output.system[0] ? `\n\n${output.system[0]}` : '');
@@ -782,7 +793,7 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
           if (part.type !== 'text' || typeof part.text !== 'string') {
             continue;
           }
-          part.text = rewriteDisplayNameMentions(config, part.text);
+          part.text = rewriteDisplayNameMentions(part.text);
         }
       }
 
@@ -794,7 +805,7 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
       processImageAttachments({
         messages: typedOutput.messages,
         workDir: ctx.directory,
-        disabledAgents: getDisabledAgents(config),
+        disabledAgents,
         log,
       });
 

src/utils/agent-variant.ts

@@ -104,19 +104,12 @@ function escapeRegExp(value: string): string {
   return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
 }
 
-/**
- * Rewrites user-facing display-name mentions (e.g. @advisor) into internal
- * agent mentions (e.g. @oracle) for runtime routing.
- */
-export function rewriteDisplayNameMentions(
-  config: PluginConfig | undefined,
-  text: string,
-): string {
-  if (!text.includes('@')) {
-    return text;
-  }
+export type DisplayNameMentionRewriter = (text: string) => string;
 
-  let rewritten = text;
+export function createDisplayNameMentionRewriter(
+  config: PluginConfig | undefined,
+): DisplayNameMentionRewriter {
+  const replacements: Array<{ regex: RegExp; internalName: string }> = [];
 
   for (const internalName of getRuntimeAgentNames(config)) {
     const displayName = getAgentOverride(config, internalName)?.displayName;
@@ -129,13 +122,45 @@ export function rewriteDisplayNameMentions(
       continue;
     }
 
-    rewritten = rewritten.replace(
-      new RegExp(`(^|[^\\w.])@${escapeRegExp(normalizedDisplayName)}\\b`, 'g'),
-      `$1@${internalName}`,
-    );
+    replacements.push({
+      regex: new RegExp(
+        `(^|[^\\w.])@${escapeRegExp(normalizedDisplayName)}\\b`,
+        'g',
+      ),
+      internalName,
+    });
   }
 
-  return rewritten;
+  if (replacements.length === 0) {
+    return (text) => text;
+  }
+
+  return (text) => {
+    if (!text.includes('@')) {
+      return text;
+    }
+
+    let rewritten = text;
+    for (const replacement of replacements) {
+      rewritten = rewritten.replace(
+        replacement.regex,
+        `$1@${replacement.internalName}`,
+      );
+    }
+
+    return rewritten;
+  };
+}
+
+/**
+ * Rewrites user-facing display-name mentions (e.g. @advisor) into internal
+ * agent mentions (e.g. @oracle) for runtime routing.
+ */
+export function rewriteDisplayNameMentions(
+  config: PluginConfig | undefined,
+  text: string,
+): string {
+  return createDisplayNameMentionRewriter(config)(text);
 }
 
 /**

src/utils/logger.test.ts

@@ -2,7 +2,13 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test';
 import * as fs from 'node:fs';
 import * as os from 'node:os';
 import * as path from 'node:path';
-import { getLogDir, initLogger, log, resetLogger } from './logger';
+import {
+  flushLoggerForTesting,
+  getLogDir,
+  initLogger,
+  log,
+  resetLogger,
+} from './logger';
 
 describe('logger', () => {
   let tmpDir: string;
@@ -15,7 +21,8 @@ describe('logger', () => {
     resetLogger();
   });
 
-  afterEach(() => {
+  afterEach(async () => {
+    await flushLoggerForTesting();
     if (origLogDir === undefined) {
       delete process.env.OPENCODE_LOG_DIR;
     } else {
@@ -37,9 +44,10 @@ describe('logger', () => {
     expect(files).toEqual(['oh-my-opencode-slim.20260416T143052.log']);
   });
 
-  test('writes log message with timestamp', () => {
+  test('writes log message with timestamp', async () => {
     initLogger('session1');
     log('timestamped message');
+    await flushLoggerForTesting();
 
     const logPath = path.join(tmpDir, 'oh-my-opencode-slim.session1.log');
     const content = fs.readFileSync(logPath, 'utf-8');
@@ -47,9 +55,10 @@ describe('logger', () => {
     expect(content).toContain('timestamped message');
   });
 
-  test('logs message with data object', () => {
+  test('logs message with data object', async () => {
     initLogger('session1');
     log('message with data', { key: 'value', number: 42 });
+    await flushLoggerForTesting();
 
     const logPath = path.join(tmpDir, 'oh-my-opencode-slim.session1.log');
     const content = fs.readFileSync(logPath, 'utf-8');
@@ -57,20 +66,22 @@ describe('logger', () => {
     expect(content).toContain('"number":42');
   });
 
-  test('logs message without extra JSON when no data', () => {
+  test('logs message without extra JSON when no data', async () => {
     initLogger('session1');
     log('message without data');
+    await flushLoggerForTesting();
 
     const logPath = path.join(tmpDir, 'oh-my-opencode-slim.session1.log');
     const content = fs.readFileSync(logPath, 'utf-8');
     expect(content.trim()).toMatch(/message without data\s*$/);
   });
 
-  test('appends multiple log entries', () => {
+  test('appends multiple log entries', async () => {
     initLogger('session1');
     log('first');
     log('second');
     log('third');
+    await flushLoggerForTesting();
 
     const logPath = path.join(tmpDir, 'oh-my-opencode-slim.session1.log');
     const lines = fs.readFileSync(logPath, 'utf-8').trim().split('\n');
@@ -80,11 +91,12 @@ describe('logger', () => {
     expect(lines[2]).toContain('third');
   });
 
-  test('initLogger called twice uses second session file', () => {
+  test('initLogger called twice uses second session file', async () => {
     initLogger('session1');
     log('from session1');
     initLogger('session2');
     log('from session2');
+    await flushLoggerForTesting();
 
     const files = fs.readdirSync(tmpDir).sort();
     expect(files).toEqual([
@@ -153,12 +165,13 @@ describe('logger', () => {
     expect(files.find((f) => f.includes('fresh'))).toBeDefined();
   });
 
-  test('handles circular references in data', () => {
+  test('handles circular references in data', async () => {
     initLogger('session1');
     const circular: any = { name: 'test' };
     circular.self = circular;
 
     expect(() => log('circular data', circular)).not.toThrow();
+    await flushLoggerForTesting();
 
     const logPath = path.join(tmpDir, 'oh-my-opencode-slim.session1.log');
     const content = fs.readFileSync(logPath, 'utf-8');
@@ -185,7 +198,7 @@ describe('logger', () => {
     }
   });
 
-  test('handles complex data structures', () => {
+  test('handles complex data structures', async () => {
     initLogger('session1');
     log('complex data', {
       nested: { deep: { value: 'test' } },
@@ -193,6 +206,7 @@ describe('logger', () => {
       boolean: true,
       null: null,
     });
+    await flushLoggerForTesting();
 
     const logPath = path.join(tmpDir, 'oh-my-opencode-slim.session1.log');
     const content = fs.readFileSync(logPath, 'utf-8');

src/utils/logger.ts

@@ -1,4 +1,5 @@
 import * as fs from 'node:fs';
+import { appendFile } from 'node:fs/promises';
 import * as os from 'node:os';
 import * as path from 'node:path';
 
@@ -7,6 +8,7 @@ const LOG_SUFFIX = '.log';
 const RETENTION_MS = 7 * 24 * 60 * 60 * 1000; // 7 days
 
 let logFile: string | null = null;
+let writeChain: Promise<void> = Promise.resolve();
 
 function getLogDir(): string {
   return (
@@ -66,18 +68,30 @@ export function initLogger(sessionId: string): void {
     // Directory creation failed — logging will silently fail
   }
   logFile = path.join(dir, `${LOG_PREFIX}${sessionId}${LOG_SUFFIX}`);
+  try {
+    fs.closeSync(fs.openSync(logFile, 'a'));
+  } catch {
+    // File creation failed — later writes will silently fail
+  }
   cleanupOldLogs(dir);
 }
 
 /** @internal Reset logger state for testing */
 export function resetLogger(): void {
   logFile = null;
+  writeChain = Promise.resolve();
+}
+
+/** @internal Wait for queued log writes in tests. */
+export async function flushLoggerForTesting(): Promise<void> {
+  await writeChain;
 }
 
 export { getLogDir };
 
 export function log(message: string, data?: unknown): void {
-  if (!logFile) return; // Uninitialized — silently no-op
+  const target = logFile;
+  if (!target) return; // Uninitialized — silently no-op
   try {
     const timestamp = new Date().toISOString();
     let dataStr = '';
@@ -89,7 +103,11 @@ export function log(message: string, data?: unknown): void {
       }
     }
     const logEntry = `[${timestamp}] ${message} ${dataStr}\n`;
-    fs.appendFileSync(logFile, logEntry);
+    writeChain = writeChain
+      .then(() => appendFile(target, logEntry))
+      .catch(() => {
+        // Silently ignore logging errors and keep future writes alive
+      });
   } catch {
     // Silently ignore logging errors
   }

src/utils/system-collapse.test.ts

@@ -57,6 +57,16 @@ describe('collapseSystemInPlace', () => {
     expect(system).toHaveLength(0);
   });
 
+  test('preserves previous empty-string cleanup behavior', () => {
+    const system = [''];
+    const output = { system };
+
+    collapseSystemInPlace(output.system);
+
+    expect(output.system).toBe(system);
+    expect(system).toHaveLength(0);
+  });
+
   test('reassignment would NOT be visible (regression guard)', () => {
     // This test documents WHY we mutate in-place and not via reassignment.
     // Simulating the broken PR #336 approach to prove it fails.

src/utils/system-collapse.ts

@@ -4,6 +4,18 @@
  * that callers holding a reference to the original array see the change.
  */
 export function collapseSystemInPlace(system: string[]): void {
+  if (system.length === 0) {
+    return;
+  }
+
+  if (system.length === 1) {
+    if (system[0]) {
+      return;
+    }
+    system.length = 0;
+    return;
+  }
+
   const joined = system.join('\n\n');
   system.length = 0;
   if (joined) {