Browse Source

Merge pull request #964 from adikpb/cleanup/dedup-helpers

refactor(utils): dedup identical helper implementations
Alvin 1 week ago
parent
commit
110d75ebbf

+ 1 - 1
src/agents/codemap.md

@@ -104,7 +104,7 @@ export function getAgentConfigs(config?: PluginConfig): Record<string, SDKAgentC
     
     // Handle display names: create both displayName and hidden alias
     if (a.displayName) {
-      entries.push([normalizeDisplayName(a.displayName), sdkConfig]);
+      entries.push([normalizeAgentName(a.displayName), sdkConfig]);
       entries.push([a.name, { ...sdkConfig, hidden: true }]);
     } else {
       entries.push([a.name, sdkConfig]);

+ 5 - 13
src/agents/index.ts

@@ -15,6 +15,7 @@ import {
   SUBAGENT_NAMES,
 } from '../config';
 import { getAgentMcpList } from '../config/agent-mcps';
+import { escapeRegExp, normalizeAgentName } from '../utils/agent-variant';
 
 import { createCouncilAgent } from './council';
 import { buildCouncillorAgents, getCouncillorSeatName } from './council-agents';
@@ -43,11 +44,6 @@ type AgentFactory = (
 const CANCEL_TASK_ALLOWED_AGENTS = new Set(['orchestrator']);
 const SAFE_AGENT_ALIAS_RE = /^[a-z][a-z0-9_-]*$/i;
 
-function normalizeDisplayName(displayName: string): string {
-  const trimmed = displayName.trim();
-  return trimmed.startsWith('@') ? trimmed.slice(1) : trimmed;
-}
-
 function getPrimaryModelFromOverride(
   override: AgentOverrideConfig | undefined,
 ): string | undefined {
@@ -142,10 +138,6 @@ function isSafeDisplayName(displayName: string): boolean {
   return SAFE_AGENT_ALIAS_RE.test(displayName);
 }
 
-function escapeRegExp(value: string): string {
-  return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
-}
-
 // Agent Configuration Helpers
 
 /**
@@ -269,7 +261,7 @@ function injectDisplayNames(
   for (const [internalName, displayName] of nameMap) {
     prompt = prompt.replace(
       new RegExp(`@${escapeRegExp(internalName)}\\b`, 'g'),
-      `@${normalizeDisplayName(displayName)}`,
+      `@${normalizeAgentName(displayName)}`,
     );
   }
 
@@ -614,7 +606,7 @@ export function createAgents(
   // Validate display names
   const usedDisplayNames = new Set<string>();
   for (const [, displayName] of displayNameMap) {
-    const normalizedDisplayName = normalizeDisplayName(displayName);
+    const normalizedDisplayName = normalizeAgentName(displayName);
     if (!isSafeDisplayName(normalizedDisplayName)) {
       throw new Error(
         `displayName '${normalizedDisplayName}' must match /^[a-z][a-z0-9_-]*$/i`,
@@ -647,7 +639,7 @@ export function createAgents(
     for (const [internalName, displayName] of displayNameMap) {
       text = text.replace(
         new RegExp(`@${escapeRegExp(internalName)}\\b`, 'g'),
-        `@${normalizeDisplayName(displayName)}`,
+        `@${normalizeAgentName(displayName)}`,
       );
     }
     return text;
@@ -747,7 +739,7 @@ export function getAgentConfigs(
     applyClassification(a.name, sdkConfig);
 
     const normalizedDisplayName = a.displayName
-      ? normalizeDisplayName(a.displayName)
+      ? normalizeAgentName(a.displayName)
       : undefined;
 
     if (normalizedDisplayName && !isInternalOnly(a.name)) {

+ 3 - 6
src/companion/updater.ts

@@ -15,6 +15,7 @@ import { homedir, platform, tmpdir } from 'node:os';
 import * as path from 'node:path';
 import { setTimeout as delay } from 'node:timers/promises';
 import type { CompanionConfig } from '../config/schema';
+import { getErrorMessage } from '../hooks/apply-patch/errors';
 import { crossSpawn } from '../utils/compat';
 import { log } from '../utils/logger';
 
@@ -260,7 +261,7 @@ async function installCompanionArchive(
     return {
       status: 'failed',
       binaryPath: finalBinaryPath,
-      error: `Failed to fetch companion archive: ${formatError(err)}`,
+      error: `Failed to fetch companion archive: ${getErrorMessage(err)}`,
     };
   } finally {
     clearTimeout(timeout);
@@ -344,7 +345,7 @@ async function installCompanionArchive(
     return {
       status: 'failed',
       binaryPath: finalBinaryPath,
-      error: `Failed to install companion: ${formatError(err)}`,
+      error: `Failed to install companion: ${getErrorMessage(err)}`,
     };
   } finally {
     if (tempDir) {
@@ -456,7 +457,3 @@ function parseSemver(version: string): [number, number, number] | null {
   if (!match) return null;
   return [Number(match[1]), Number(match[2]), Number(match[3])];
 }
-
-function formatError(err: unknown): string {
-  return err instanceof Error ? err.message : String(err);
-}

+ 2 - 3
src/hooks/task-session-manager/tool-execute-hooks.ts

@@ -18,13 +18,12 @@ import {
 } from '../../utils';
 import { isRecord as isObjectRecord } from '../../utils/guards';
 import { log } from '../../utils/logger';
+import { SESSION_ID_PATTERN } from '../../utils/session';
 import { isMissingRememberedSessionError } from './board-injection';
 import type { PendingTaskCall } from './pending-call-tracker';
 import { normalizeLateCancelledTaskOutput } from './status-utils';
 import { extractReadFiles } from './task-context-tracker';
 
-const RAW_SESSION_ID_PATTERN = /^ses_[A-Za-z0-9_-]+$/;
-
 interface TaskArgs {
   description?: unknown;
   prompt?: unknown;
@@ -122,7 +121,7 @@ export async function handleToolExecuteBefore(
 
       if (knownManagedTask) {
         delete args.task_id;
-      } else if (RAW_SESSION_ID_PATTERN.test(requested)) {
+      } else if (SESSION_ID_PATTERN.test(requested)) {
         pendingCall.resumedTaskId = requested;
       } else {
         delete args.task_id;

+ 7 - 10
src/tools/cancel-task.ts

@@ -7,7 +7,12 @@ import type { BackgroundJobStore } from '../utils/background-job-store';
 import { isRecord as isObjectRecord } from '../utils/guards';
 import { log } from '../utils/logger';
 import { getClient } from '../utils/opencode-client';
-import { abortSessionWithTimeout, withTimeout } from '../utils/session';
+import { delay } from '../utils/polling';
+import {
+  abortSessionWithTimeout,
+  SESSION_ID_PATTERN,
+  withTimeout,
+} from '../utils/session';
 
 const z = tool.schema;
 
@@ -74,7 +79,7 @@ Use only for obsolete, wrong, conflicting, or user-requested cancellation. Accep
         cancellationRequested: job?.cancellationRequested,
       });
       if (!job) {
-        if (isSessionID(requested)) {
+        if (SESSION_ID_PATTERN.test(requested)) {
           if (requested === parentSessionID) {
             log('[cancel-task] rejected parent session cancellation', {
               parentSessionID,
@@ -361,14 +366,6 @@ async function getSessionStatus(
   }
 }
 
-function delay(ms: number): Promise<void> {
-  return new Promise((resolve) => setTimeout(resolve, ms));
-}
-
-function isSessionID(value: string): boolean {
-  return /^ses_[\w-]+$/.test(value);
-}
-
 function normalizeCancelReason(reason?: string): string {
   const normalized = reason?.replace(/\s+/g, ' ').trim();
   return normalized ? `cancelled: ${normalized}` : 'cancelled';

+ 1 - 1
src/utils/agent-variant.ts

@@ -63,7 +63,7 @@ export function resolveRuntimeAgentName(
   return normalized;
 }
 
-function escapeRegExp(value: string): string {
+export function escapeRegExp(value: string): string {
   return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
 }
 

+ 2 - 0
src/utils/session.ts

@@ -7,6 +7,8 @@ import { log } from './logger';
 
 export const SESSION_ABORT_TIMEOUT_MS = 1_000;
 
+export const SESSION_ID_PATTERN = /^ses_[A-Za-z0-9_-]+$/;
+
 export class OperationTimeoutError extends Error {
   constructor(message: string) {
     super(message);