From 669b352d36d8e58bd8f4d9ce7a3d3f0fb99c38ac Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 7 Apr 2026 23:51:06 +0100 Subject: [PATCH] refactor: dedupe auto-reply trimmed readers --- src/auto-reply/command-auth.ts | 15 ++++++------ src/auto-reply/commands-args.ts | 13 +++++++---- src/auto-reply/heartbeat.ts | 3 ++- src/auto-reply/model-runtime.ts | 23 +++++++++++-------- src/auto-reply/reply/commands-acp/targets.ts | 10 ++++---- src/auto-reply/reply/commands-session.ts | 2 +- .../reply/commands-subagents/action-spawn.ts | 8 +++---- .../reply/commands-subagents/shared.ts | 2 +- src/auto-reply/reply/commands-tts.ts | 10 ++++++-- .../reply/directive-handling.model.ts | 4 ++-- src/auto-reply/reply/dispatch-acp.ts | 5 +++- src/auto-reply/reply/reply-elevated.ts | 2 +- src/auto-reply/status.ts | 10 ++++---- .../command-auth-registry-fixture.ts | 9 +++++--- 14 files changed, 68 insertions(+), 48 deletions(-) diff --git a/src/auto-reply/command-auth.ts b/src/auto-reply/command-auth.ts index 7cdafaee661..4632dfb731e 100644 --- a/src/auto-reply/command-auth.ts +++ b/src/auto-reply/command-auth.ts @@ -5,6 +5,7 @@ import type { OpenClawConfig } from "../config/config.js"; import { normalizeLowercaseStringOrEmpty, normalizeOptionalLowercaseString, + normalizeOptionalString, } from "../shared/string-coerce.js"; import { normalizeStringEntries } from "../shared/string-normalization.js"; import { @@ -256,7 +257,7 @@ function buildProviderAllowFromResolution(params: { function describeAllowFromResolutionError(err: unknown): string { if (err instanceof Error) { - const name = err.name.trim(); + const name = normalizeOptionalString(err.name) ?? ""; return name || "Error"; } return "unknown_error"; @@ -275,7 +276,7 @@ function resolveOwnerAllowFromList(params: { } const filtered: string[] = []; for (const entry of raw) { - const trimmed = String(entry ?? "").trim(); + const trimmed = normalizeOptionalString(String(entry ?? "")) ?? ""; if (!trimmed) { continue; } @@ -466,7 +467,7 @@ function shouldUseFromAsSenderFallback(params: { from?: string | null; chatType?: string | null; }): boolean { - const from = (params.from ?? "").trim(); + const from = normalizeOptionalString(params.from) ?? ""; if (!from) { return false; } @@ -490,7 +491,7 @@ function resolveSenderCandidates(params: { const { plugin, cfg, accountId } = params; const candidates: string[] = []; const pushCandidate = (value?: string | null) => { - const trimmed = (value ?? "").trim(); + const trimmed = normalizeOptionalString(value) ?? ""; if (!trimmed) { return; } @@ -527,7 +528,7 @@ function resolveFallbackAllowFrom(params: { providerId?: ChannelId; accountId?: string | null; }): Array { - const providerId = params.providerId?.trim(); + const providerId = normalizeOptionalString(params.providerId); if (!providerId) { return []; } @@ -629,8 +630,8 @@ export function resolveCommandAuthorization(params: { cfg, ); const plugin = providerId ? getChannelPlugin(providerId) : undefined; - const from = (ctx.From ?? "").trim(); - const to = (ctx.To ?? "").trim(); + const from = normalizeOptionalString(ctx.From) ?? ""; + const to = normalizeOptionalString(ctx.To) ?? ""; const commandsAllowFromConfigured = Boolean( cfg.commands?.allowFrom && typeof cfg.commands.allowFrom === "object", ); diff --git a/src/auto-reply/commands-args.ts b/src/auto-reply/commands-args.ts index 7ebad62336e..cafdf503cd3 100644 --- a/src/auto-reply/commands-args.ts +++ b/src/auto-reply/commands-args.ts @@ -1,4 +1,7 @@ -import { normalizeOptionalLowercaseString } from "../shared/string-coerce.js"; +import { + normalizeOptionalLowercaseString, + normalizeOptionalString, +} from "../shared/string-coerce.js"; import type { CommandArgValues } from "./commands-registry.types.js"; export type CommandArgsFormatter = (values: CommandArgValues) => string | undefined; @@ -9,13 +12,13 @@ function normalizeArgValue(value: unknown): string | undefined { } let text: string; if (typeof value === "string") { - text = value.trim(); + text = normalizeOptionalString(value) ?? ""; } else if (typeof value === "number" || typeof value === "boolean" || typeof value === "bigint") { - text = String(value).trim(); + text = normalizeOptionalString(String(value)) ?? ""; } else if (typeof value === "symbol") { - text = value.toString().trim(); + text = normalizeOptionalString(value.toString()) ?? ""; } else if (typeof value === "function") { - text = value.toString().trim(); + text = normalizeOptionalString(value.toString()) ?? ""; } else { // Objects and arrays text = JSON.stringify(value); diff --git a/src/auto-reply/heartbeat.ts b/src/auto-reply/heartbeat.ts index e2ffa71b6c4..91d4baba3fc 100644 --- a/src/auto-reply/heartbeat.ts +++ b/src/auto-reply/heartbeat.ts @@ -1,4 +1,5 @@ import { parseDurationMs } from "../cli/parse-duration.js"; +import { normalizeOptionalString } from "../shared/string-coerce.js"; import { escapeRegExp } from "../utils.js"; import { HEARTBEAT_TOKEN } from "./tokens.js"; @@ -60,7 +61,7 @@ export function isHeartbeatContentEffectivelyEmpty(content: string | undefined | } export function resolveHeartbeatPrompt(raw?: string): string { - const trimmed = typeof raw === "string" ? raw.trim() : ""; + const trimmed = normalizeOptionalString(raw) ?? ""; return trimmed || HEARTBEAT_PROMPT; } diff --git a/src/auto-reply/model-runtime.ts b/src/auto-reply/model-runtime.ts index 0e5bd704876..f12b07b3cd9 100644 --- a/src/auto-reply/model-runtime.ts +++ b/src/auto-reply/model-runtime.ts @@ -1,9 +1,12 @@ import type { SessionEntry } from "../config/sessions.js"; -import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js"; +import { + normalizeLowercaseStringOrEmpty, + normalizeOptionalString, +} from "../shared/string-coerce.js"; export function formatProviderModelRef(providerRaw: string, modelRaw: string): string { - const provider = String(providerRaw ?? "").trim(); - const model = String(modelRaw ?? "").trim(); + const provider = normalizeOptionalString(providerRaw) ?? ""; + const model = normalizeOptionalString(modelRaw) ?? ""; if (!provider) { return model; } @@ -27,7 +30,7 @@ type ModelRef = { }; function normalizeModelWithinProvider(provider: string, modelRaw: string): string { - const model = String(modelRaw ?? "").trim(); + const model = normalizeOptionalString(modelRaw) ?? ""; if (!provider || !model) { return model; } @@ -46,11 +49,11 @@ function normalizeModelRef( fallbackProvider: string, parseEmbeddedProvider = false, ): ModelRef { - const trimmed = String(rawModel ?? "").trim(); + const trimmed = normalizeOptionalString(rawModel) ?? ""; const slashIndex = parseEmbeddedProvider ? trimmed.indexOf("/") : -1; if (slashIndex > 0) { - const provider = trimmed.slice(0, slashIndex).trim(); - const model = trimmed.slice(slashIndex + 1).trim(); + const provider = normalizeOptionalString(trimmed.slice(0, slashIndex)) ?? ""; + const model = normalizeOptionalString(trimmed.slice(slashIndex + 1)) ?? ""; if (provider && model) { return { provider, @@ -59,7 +62,7 @@ function normalizeModelRef( }; } } - const provider = String(fallbackProvider ?? "").trim(); + const provider = normalizeOptionalString(fallbackProvider) ?? ""; const dedupedModel = normalizeModelWithinProvider(provider, trimmed); return { provider, @@ -78,8 +81,8 @@ export function resolveSelectedAndActiveModel(params: { activeDiffers: boolean; } { const selected = normalizeModelRef(params.selectedModel, params.selectedProvider); - const runtimeModel = params.sessionEntry?.model?.trim(); - const runtimeProvider = params.sessionEntry?.modelProvider?.trim(); + const runtimeModel = normalizeOptionalString(params.sessionEntry?.model); + const runtimeProvider = normalizeOptionalString(params.sessionEntry?.modelProvider); const active = runtimeModel ? normalizeModelRef(runtimeModel, runtimeProvider || selected.provider, !runtimeProvider) diff --git a/src/auto-reply/reply/commands-acp/targets.ts b/src/auto-reply/reply/commands-acp/targets.ts index 4e8e8104e30..1b85d493b78 100644 --- a/src/auto-reply/reply/commands-acp/targets.ts +++ b/src/auto-reply/reply/commands-acp/targets.ts @@ -24,7 +24,7 @@ async function resolveSessionKeyByToken(token: string): Promise { params, timeoutMs: 8_000, }); - const key = typeof resolved?.key === "string" ? resolved.key.trim() : ""; + const key = normalizeOptionalString(resolved?.key) ?? ""; if (key) { return key; } @@ -36,11 +36,9 @@ async function resolveSessionKeyByToken(token: string): Promise { } export function resolveBoundAcpThreadSessionKey(params: HandleCommandsParams): string | undefined { - const commandTargetSessionKey = - typeof params.ctx.CommandTargetSessionKey === "string" - ? params.ctx.CommandTargetSessionKey.trim() - : ""; - const activeSessionKey = commandTargetSessionKey || params.sessionKey.trim(); + const commandTargetSessionKey = normalizeOptionalString(params.ctx.CommandTargetSessionKey) ?? ""; + const activeSessionKey = + commandTargetSessionKey || (normalizeOptionalString(params.sessionKey) ?? ""); const bindingContext = resolveAcpCommandBindingContext(params); return resolveEffectiveResetTargetSessionKey({ cfg: params.cfg, diff --git a/src/auto-reply/reply/commands-session.ts b/src/auto-reply/reply/commands-session.ts index de8d1c99347..23d7a47e404 100644 --- a/src/auto-reply/reply/commands-session.ts +++ b/src/auto-reply/reply/commands-session.ts @@ -81,7 +81,7 @@ function resolveSessionBindingLastActivityAt(binding: SessionBindingRecord): num function resolveSessionBindingBoundBy(binding: SessionBindingRecord): string { const raw = binding.metadata?.boundBy; - return typeof raw === "string" ? raw.trim() : ""; + return normalizeOptionalString(raw) ?? ""; } type UpdatedLifecycleBinding = { diff --git a/src/auto-reply/reply/commands-subagents/action-spawn.ts b/src/auto-reply/reply/commands-subagents/action-spawn.ts index bb4b58bd865..9cd8f5908dd 100644 --- a/src/auto-reply/reply/commands-subagents/action-spawn.ts +++ b/src/auto-reply/reply/commands-subagents/action-spawn.ts @@ -1,4 +1,5 @@ import { spawnSubagentDirect } from "../../../agents/subagent-spawn.js"; +import { normalizeOptionalString } from "../../../shared/string-coerce.js"; import type { CommandHandlerResult } from "../commands-types.js"; import { type SubagentsCommandContext, stopWithText } from "./shared.js"; @@ -29,10 +30,9 @@ export async function handleSubagentsSpawnAction( ); } - const commandTo = typeof params.command.to === "string" ? params.command.to.trim() : ""; - const originatingTo = - typeof params.ctx.OriginatingTo === "string" ? params.ctx.OriginatingTo.trim() : ""; - const fallbackTo = typeof params.ctx.To === "string" ? params.ctx.To.trim() : ""; + const commandTo = normalizeOptionalString(params.command.to) ?? ""; + const originatingTo = normalizeOptionalString(params.ctx.OriginatingTo) ?? ""; + const fallbackTo = normalizeOptionalString(params.ctx.To) ?? ""; const normalizedTo = originatingTo || commandTo || fallbackTo || undefined; const result = await spawnSubagentDirect( diff --git a/src/auto-reply/reply/commands-subagents/shared.ts b/src/auto-reply/reply/commands-subagents/shared.ts index d5e8504c964..68f63c8f996 100644 --- a/src/auto-reply/reply/commands-subagents/shared.ts +++ b/src/auto-reply/reply/commands-subagents/shared.ts @@ -338,7 +338,7 @@ export async function resolveFocusTargetSession(params: { method: "sessions.resolve", params: attempt, }); - const key = typeof resolved?.key === "string" ? resolved.key.trim() : ""; + const key = normalizeOptionalString(resolved?.key) ?? ""; if (!key) { continue; } diff --git a/src/auto-reply/reply/commands-tts.ts b/src/auto-reply/reply/commands-tts.ts index db249ee1738..46a054d8280 100644 --- a/src/auto-reply/reply/commands-tts.ts +++ b/src/auto-reply/reply/commands-tts.ts @@ -1,5 +1,8 @@ import { logVerbose } from "../../globals.js"; -import { normalizeOptionalLowercaseString } from "../../shared/string-coerce.js"; +import { + normalizeOptionalLowercaseString, + normalizeOptionalString, +} from "../../shared/string-coerce.js"; import { canonicalizeSpeechProviderId, getSpeechProvider, @@ -47,7 +50,10 @@ function parseTtsCommand(normalized: string): ParsedTtsCommand | null { return { action: "status", args: "" }; } const [action, ...tail] = rest.split(/\s+/); - return { action: normalizeOptionalLowercaseString(action) ?? "", args: tail.join(" ").trim() }; + return { + action: normalizeOptionalLowercaseString(action) ?? "", + args: normalizeOptionalString(tail.join(" ")) ?? "", + }; } function formatAttemptDetails(attempts: TtsAttemptDetail[] | undefined): string | undefined { diff --git a/src/auto-reply/reply/directive-handling.model.ts b/src/auto-reply/reply/directive-handling.model.ts index e875fdca9af..b9a4ce7e714 100644 --- a/src/auto-reply/reply/directive-handling.model.ts +++ b/src/auto-reply/reply/directive-handling.model.ts @@ -38,7 +38,7 @@ function pushUniqueCatalogEntry(params: { fallbackNameToId: boolean; }) { const provider = normalizeProviderId(params.provider); - const id = String(params.id ?? "").trim(); + const id = normalizeOptionalString(params.id) ?? ""; if (!provider || !id) { return; } @@ -83,7 +83,7 @@ function buildModelPickerCatalog(params: { }; const pushRaw = (raw?: string) => { - const value = String(raw ?? "").trim(); + const value = normalizeOptionalString(raw) ?? ""; if (!value) { return; } diff --git a/src/auto-reply/reply/dispatch-acp.ts b/src/auto-reply/reply/dispatch-acp.ts index 7f40f253e2d..71b6d43349c 100644 --- a/src/auto-reply/reply/dispatch-acp.ts +++ b/src/auto-reply/reply/dispatch-acp.ts @@ -327,7 +327,10 @@ export async function tryDispatchAcpReply(params: { const shouldEmitResolvedIdentityNotice = !params.suppressUserDelivery && identityPendingBeforeTurn && - (Boolean(params.ctx.MessageThreadId != null && String(params.ctx.MessageThreadId).trim()) || + (Boolean( + params.ctx.MessageThreadId != null && + (normalizeOptionalString(String(params.ctx.MessageThreadId)) ?? ""), + ) || (await hasBoundConversationForSession({ cfg: params.cfg, sessionKey: canonicalSessionKey, diff --git a/src/auto-reply/reply/reply-elevated.ts b/src/auto-reply/reply/reply-elevated.ts index 86eba86e990..01e08d3147f 100644 --- a/src/auto-reply/reply/reply-elevated.ts +++ b/src/auto-reply/reply/reply-elevated.ts @@ -46,7 +46,7 @@ function resolveAllowFromFormatter(params: { accountId: params.accountId, allowFrom: values, }) - .map((entry) => String(entry).trim()) + .map((entry) => normalizeOptionalString(String(entry)) ?? "") .filter(Boolean); } diff --git a/src/auto-reply/status.ts b/src/auto-reply/status.ts index 720f2ea89d6..fb83a8180c0 100644 --- a/src/auto-reply/status.ts +++ b/src/auto-reply/status.ts @@ -34,6 +34,7 @@ import { resolveAgentIdFromSessionKey } from "../routing/session-key.js"; import { normalizeLowercaseStringOrEmpty, normalizeOptionalLowercaseString, + normalizeOptionalString, } from "../shared/string-coerce.js"; import { resolveStatusTtsSnapshot } from "../tts/status-config.js"; import { @@ -457,9 +458,8 @@ export function buildStatusMessage(args: StatusArgs): string { let activeModel = modelRefs.active.model; let contextLookupProvider: string | undefined = activeProvider; let contextLookupModel = activeModel; - const runtimeModelRaw = typeof entry?.model === "string" ? entry.model.trim() : ""; - const runtimeProviderRaw = - typeof entry?.modelProvider === "string" ? entry.modelProvider.trim() : ""; + const runtimeModelRaw = normalizeOptionalString(entry?.model) ?? ""; + const runtimeProviderRaw = normalizeOptionalString(entry?.modelProvider) ?? ""; if (runtimeModelRaw && !runtimeProviderRaw && runtimeModelRaw.includes("/")) { const slashIndex = runtimeModelRaw.indexOf("/"); @@ -468,7 +468,9 @@ export function buildStatusMessage(args: StatusArgs): string { const fallbackMatchesRuntimeModel = initialFallbackState.active && normalizeLowercaseStringOrEmpty(runtimeModelRaw) === - normalizeLowercaseStringOrEmpty(String(entry?.fallbackNoticeActiveModel ?? "").trim()); + normalizeLowercaseStringOrEmpty( + normalizeOptionalString(String(entry?.fallbackNoticeActiveModel ?? "")) ?? "", + ); const runtimeMatchesSelectedModel = normalizeLowercaseStringOrEmpty(runtimeModelRaw) === normalizeLowercaseStringOrEmpty(modelRefs.selected.label || "unknown"); diff --git a/src/auto-reply/test-helpers/command-auth-registry-fixture.ts b/src/auto-reply/test-helpers/command-auth-registry-fixture.ts index 41b5d623f11..ac8f4c1efd5 100644 --- a/src/auto-reply/test-helpers/command-auth-registry-fixture.ts +++ b/src/auto-reply/test-helpers/command-auth-registry-fixture.ts @@ -1,12 +1,15 @@ import { afterEach, beforeEach } from "vitest"; import { normalizeE164 } from "../../plugin-sdk/account-resolution.js"; import { setActivePluginRegistry } from "../../plugins/runtime.js"; -import { lowercasePreservingWhitespace } from "../../shared/string-coerce.js"; +import { + lowercasePreservingWhitespace, + normalizeOptionalString, +} from "../../shared/string-coerce.js"; import { createOutboundTestPlugin, createTestRegistry } from "../../test-utils/channel-plugins.js"; function formatDiscordAllowFromEntries(allowFrom: Array): string[] { return allowFrom - .map((entry) => String(entry).trim()) + .map((entry) => normalizeOptionalString(String(entry)) ?? "") .filter(Boolean) .map((entry) => entry.replace(/^(discord|user|pk):/i, "").replace(/^<@!?(\d+)>$/, "$1")) .map((entry) => lowercasePreservingWhitespace(entry)); @@ -14,7 +17,7 @@ function formatDiscordAllowFromEntries(allowFrom: Array): strin function normalizePhoneAllowFromEntries(allowFrom: Array): string[] { return allowFrom - .map((entry) => String(entry).trim()) + .map((entry) => normalizeOptionalString(String(entry)) ?? "") .filter((entry): entry is string => Boolean(entry)) .map((entry) => { if (entry === "*") {