From 84cd4c5ffac193ef9903c03363585c9ddafc2e70 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 22 Oct 2026 21:50:19 +0000 Subject: [PATCH 1/8] lint: getXumHome joins the host modules of local/no-host-imports --- .oxlintrc.json | 3 ++- eslint.config.mjs | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.oxlintrc.json b/.oxlintrc.json index 38457849962..cdbde010985 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -1250,7 +1250,8 @@ { "module": "@/node/runtime/runtimeHelpers", "names": ["createRuntimeForWorkspace", "createRuntimeContextForWorkspace"] - } + }, + { "module": "@/common/constants/paths", "names": ["getXumHome"] } ], "allow": { "src/node/services/agentSession.ts": ["@/node/runtime/runtimeHelpers", "fs/promises"], diff --git a/eslint.config.mjs b/eslint.config.mjs index 0bd1faf4892..9497530aa0e 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -117,6 +117,9 @@ const HOST_MODULES = [ module: "@/node/runtime/runtimeHelpers", names: ["createRuntimeForWorkspace", "createRuntimeContextForWorkspace"], }, + // getXumHome() reads XUM_ROOT and stats ~/.xum (paths.ts). Phase 5 gives loop code the + // Xum home through HostPaths instead. Its other exports are pure and stay allowed. + { module: "@/common/constants/paths", names: ["getXumHome"] }, ]; // File -> the host modules it still imports. Phase plans list the pairs they add or remove. const HOST_IMPORT_ALLOW = { From ea35e9bcdfa5633f897ba5d38cb9ff4386fa13ad Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 22 Oct 2026 21:54:56 +0000 Subject: [PATCH 2/8] refactor: web_fetch and blockedTargets take isIP from a pure helper --- .oxlintrc.json | 1 - eslint.config.mjs | 1 - scripts/noHostImports.test.ts | 1 - src/node/services/tools/web_fetch.ts | 8 ++++---- src/node/utils/ipAddress.test.ts | 21 +++++++++++++++++++++ src/node/utils/ipAddress.ts | 16 ++++++++++++++++ src/node/utils/network/blockedTargets.ts | 6 +++--- 7 files changed, 44 insertions(+), 10 deletions(-) create mode 100644 src/node/utils/ipAddress.test.ts create mode 100644 src/node/utils/ipAddress.ts diff --git a/.oxlintrc.json b/.oxlintrc.json index cdbde010985..3f8db231245 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -1283,7 +1283,6 @@ "src/node/services/tools/skillFileUtils.ts": ["fs/promises"], "src/node/services/tools/skillsCatalogFetch.ts": ["fs/promises", "os"], "src/node/services/tools/todo.ts": ["@/node/utils/writeFileAtomic", "fs/promises"], - "src/node/services/tools/web_fetch.ts": ["net"], "src/node/services/tools/xum_agents_path.ts": ["fs/promises"], "src/node/services/tools/xum_agents_read.ts": ["fs/promises"], "src/node/services/tools/xum_agents_write.ts": ["fs/promises"] diff --git a/eslint.config.mjs b/eslint.config.mjs index 9497530aa0e..edd6e9ffa62 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -149,7 +149,6 @@ const HOST_IMPORT_ALLOW = { "src/node/services/tools/skillFileUtils.ts": ["fs/promises"], "src/node/services/tools/skillsCatalogFetch.ts": ["fs/promises", "os"], "src/node/services/tools/todo.ts": ["@/node/utils/writeFileAtomic", "fs/promises"], - "src/node/services/tools/web_fetch.ts": ["net"], "src/node/services/tools/xum_agents_path.ts": ["fs/promises"], "src/node/services/tools/xum_agents_read.ts": ["fs/promises"], "src/node/services/tools/xum_agents_write.ts": ["fs/promises"], diff --git a/scripts/noHostImports.test.ts b/scripts/noHostImports.test.ts index d178abf0103..82f6cded28f 100644 --- a/scripts/noHostImports.test.ts +++ b/scripts/noHostImports.test.ts @@ -146,7 +146,6 @@ const FROZEN_ALLOW_PAIRS = [ "src/node/services/tools/skillsCatalogFetch.ts os", "src/node/services/tools/todo.ts @/node/utils/writeFileAtomic", "src/node/services/tools/todo.ts fs/promises", - "src/node/services/tools/web_fetch.ts net", "src/node/services/tools/xum_agents_path.ts fs/promises", "src/node/services/tools/xum_agents_read.ts fs/promises", "src/node/services/tools/xum_agents_write.ts fs/promises", diff --git a/src/node/services/tools/web_fetch.ts b/src/node/services/tools/web_fetch.ts index c93b3d3cb94..7353a70518e 100644 --- a/src/node/services/tools/web_fetch.ts +++ b/src/node/services/tools/web_fetch.ts @@ -2,7 +2,6 @@ import { tool } from "ai"; import { Window } from "happy-dom"; import { Readability } from "@mozilla/readability"; import TurndownService from "turndown"; -import * as net from "node:net"; import type { WebFetchToolResult } from "@/common/types/tools"; import { shellQuote } from "@/common/utils/shell"; import type { ToolConfiguration, ToolFactory } from "@/common/utils/tools/tools"; @@ -19,6 +18,7 @@ import { getErrorMessage } from "@/common/utils/errors"; import { assert } from "@/common/utils/assert"; import type { Runtime } from "@/node/runtime/Runtime"; import { log } from "@/node/services/log"; +import { isIP } from "@/node/utils/ipAddress"; import { isBlockedHostname, isBlockedIpAddress, @@ -188,7 +188,7 @@ function parseResolvedAddresses(output: string): string[] { } const normalizedAddress = normalizeHostname(value); - if (net.isIP(normalizedAddress) === 0) { + if (isIP(normalizedAddress) === 0) { throw new WebFetchValidationError(WEB_FETCH_RESOLVE_ERROR); } @@ -292,7 +292,7 @@ function buildConnectToPin(url: URL, address: string): string { if (url.hostname.length === 0 || /[:[\]]/.test(url.hostname) || !/^\d+$/.test(port)) { throw new WebFetchValidationError(WEB_FETCH_BLOCKED_TARGET_ERROR); } - const routedAddress = net.isIP(address) === 6 ? `[${address}]` : address; + const routedAddress = isIP(address) === 6 ? `[${address}]` : address; return `${url.hostname}:${port}:${routedAddress}:${port}`; } @@ -316,7 +316,7 @@ async function assertWebFetchTargetAllowed( throw new WebFetchValidationError(WEB_FETCH_BLOCKED_TARGET_ERROR); } - if (net.isIP(hostname) !== 0) { + if (isIP(hostname) !== 0) { if (isBlockedIpAddress(hostname)) { throw new WebFetchValidationError(WEB_FETCH_BLOCKED_TARGET_ERROR); } diff --git a/src/node/utils/ipAddress.test.ts b/src/node/utils/ipAddress.test.ts new file mode 100644 index 00000000000..062f19802c5 --- /dev/null +++ b/src/node/utils/ipAddress.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, test } from "bun:test"; +import { isIP } from "./ipAddress"; + +describe("isIP", () => { + // web_fetch.ts relies on these answers: 0 rejects a DNS answer, 6 brackets a + // connect address, and a zone index (fe80::1%eth0) still counts as IPv6. + test.each([ + ["127.0.0.1", 4], + ["255.255.255.255", 4], + ["::1", 6], + ["2001:db8::1", 6], + ["fe80::1%eth0", 6], + ["::ffff:127.0.0.1", 6], + ["example.com", 0], + ["256.0.0.1", 0], + ["[::1]", 0], + ["", 0], + ] as const)("%p is version %p", (input, version) => { + expect(isIP(input)).toBe(version); + }); +}); diff --git a/src/node/utils/ipAddress.ts b/src/node/utils/ipAddress.ts new file mode 100644 index 00000000000..889164bbf95 --- /dev/null +++ b/src/node/utils/ipAddress.ts @@ -0,0 +1,16 @@ +import * as net from "node:net"; + +/** + * The IP version of a literal address: 4, 6, or 0 when `input` is not one. + * + * net.isIP is pure (no file, socket or DNS access), but agent-loop files may not import + * `net` (local/no-host-imports, which cannot tell its pure helpers from sockets). Loop + * code and its helpers take isIP from here instead. + */ +export function isIP(input: string): 0 | 4 | 6 { + const version = net.isIP(input); + if (version !== 0 && version !== 4 && version !== 6) { + throw new Error(`net.isIP returned an unknown IP version: ${version}`); + } + return version; +} diff --git a/src/node/utils/network/blockedTargets.ts b/src/node/utils/network/blockedTargets.ts index 3f616e892c0..0c6e78c61f7 100644 --- a/src/node/utils/network/blockedTargets.ts +++ b/src/node/utils/network/blockedTargets.ts @@ -1,4 +1,4 @@ -import * as net from "node:net"; +import { isIP } from "@/node/utils/ipAddress"; /** * Pure SSRF target validators shared by outbound fetchers (web_fetch, MCP icon @@ -23,7 +23,7 @@ export function normalizeHostname(hostname: string): string { } function parseIpv4Octets(address: string): number[] | null { - if (net.isIP(address) !== 4) { + if (isIP(address) !== 4) { return null; } @@ -52,7 +52,7 @@ function normalizeIpv6Address(address: string): string { function parseIpv6Segments(address: string): number[] | null { let normalized = normalizeIpv6Address(address); - if (net.isIP(normalized) !== 6) { + if (isIP(normalized) !== 6) { return null; } From 27de2e580986487cf88cd1b1ed0449c2b624c046 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 22 Oct 2026 22:16:58 +0000 Subject: [PATCH 3/8] refactor: agent-loop tools branch on Runtime.location instead of instanceof --- .oxlintrc.json | 4 +- eslint.config.mjs | 4 +- scripts/noHostImports.test.ts | 3 - src/node/runtime/DevcontainerRuntime.ts | 5 ++ src/node/runtime/LocalBaseRuntime.ts | 6 ++ src/node/runtime/RemoteRuntime.ts | 6 ++ src/node/runtime/Runtime.ts | 21 +++++++ src/node/runtime/runtimeLocation.test.ts | 55 +++++++++++++++++++ .../services/tools/agent_skill_read_file.ts | 5 +- src/node/services/tools/bash.ts | 6 +- src/node/services/tools/skillFileHint.ts | 5 +- 11 files changed, 102 insertions(+), 18 deletions(-) create mode 100644 src/node/runtime/runtimeLocation.test.ts diff --git a/.oxlintrc.json b/.oxlintrc.json index 3f8db231245..f78745a5a72 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -1267,9 +1267,8 @@ ], "src/node/services/tools/agent_skill_delete.ts": ["fs/promises"], "src/node/services/tools/agent_skill_list.ts": ["fs/promises", "os"], - "src/node/services/tools/agent_skill_read_file.ts": ["@/node/runtime/RemoteRuntime"], "src/node/services/tools/agent_skill_write.ts": ["fs/promises"], - "src/node/services/tools/bash.ts": ["@/node/runtime/LocalBaseRuntime", "fs"], + "src/node/services/tools/bash.ts": ["fs"], "src/node/services/tools/notify.ts": ["electron"], "src/node/services/tools/review_pane.ts": [ "@/node/utils/writeFileAtomic", @@ -1279,7 +1278,6 @@ "@/node/utils/writeFileAtomic", "fs/promises" ], - "src/node/services/tools/skillFileHint.ts": ["@/node/runtime/RemoteRuntime"], "src/node/services/tools/skillFileUtils.ts": ["fs/promises"], "src/node/services/tools/skillsCatalogFetch.ts": ["fs/promises", "os"], "src/node/services/tools/todo.ts": ["@/node/utils/writeFileAtomic", "fs/promises"], diff --git a/eslint.config.mjs b/eslint.config.mjs index edd6e9ffa62..439950dc3e2 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -136,16 +136,14 @@ const HOST_IMPORT_ALLOW = { ], "src/node/services/tools/agent_skill_delete.ts": ["fs/promises"], "src/node/services/tools/agent_skill_list.ts": ["fs/promises", "os"], - "src/node/services/tools/agent_skill_read_file.ts": ["@/node/runtime/RemoteRuntime"], "src/node/services/tools/agent_skill_write.ts": ["fs/promises"], - "src/node/services/tools/bash.ts": ["@/node/runtime/LocalBaseRuntime", "fs"], + "src/node/services/tools/bash.ts": ["fs"], "src/node/services/tools/notify.ts": ["electron"], "src/node/services/tools/review_pane.ts": ["@/node/utils/writeFileAtomic", "fs/promises"], "src/node/services/tools/shared/configReadWrite.ts": [ "@/node/utils/writeFileAtomic", "fs/promises", ], - "src/node/services/tools/skillFileHint.ts": ["@/node/runtime/RemoteRuntime"], "src/node/services/tools/skillFileUtils.ts": ["fs/promises"], "src/node/services/tools/skillsCatalogFetch.ts": ["fs/promises", "os"], "src/node/services/tools/todo.ts": ["@/node/utils/writeFileAtomic", "fs/promises"], diff --git a/scripts/noHostImports.test.ts b/scripts/noHostImports.test.ts index 82f6cded28f..a3701388fb0 100644 --- a/scripts/noHostImports.test.ts +++ b/scripts/noHostImports.test.ts @@ -131,16 +131,13 @@ const FROZEN_ALLOW_PAIRS = [ "src/node/services/tools/agent_skill_delete.ts fs/promises", "src/node/services/tools/agent_skill_list.ts fs/promises", "src/node/services/tools/agent_skill_list.ts os", - "src/node/services/tools/agent_skill_read_file.ts @/node/runtime/RemoteRuntime", "src/node/services/tools/agent_skill_write.ts fs/promises", - "src/node/services/tools/bash.ts @/node/runtime/LocalBaseRuntime", "src/node/services/tools/bash.ts fs", "src/node/services/tools/notify.ts electron", "src/node/services/tools/review_pane.ts @/node/utils/writeFileAtomic", "src/node/services/tools/review_pane.ts fs/promises", "src/node/services/tools/shared/configReadWrite.ts @/node/utils/writeFileAtomic", "src/node/services/tools/shared/configReadWrite.ts fs/promises", - "src/node/services/tools/skillFileHint.ts @/node/runtime/RemoteRuntime", "src/node/services/tools/skillFileUtils.ts fs/promises", "src/node/services/tools/skillsCatalogFetch.ts fs/promises", "src/node/services/tools/skillsCatalogFetch.ts os", diff --git a/src/node/runtime/DevcontainerRuntime.ts b/src/node/runtime/DevcontainerRuntime.ts index add8350f98d..284ac5b350f 100644 --- a/src/node/runtime/DevcontainerRuntime.ts +++ b/src/node/runtime/DevcontainerRuntime.ts @@ -18,6 +18,7 @@ import type { EnsureReadyOptions, FileStat, ReadFileOptions, + RuntimeLocation, } from "./Runtime"; import { RuntimeError, WORKSPACE_REPO_MISSING_ERROR } from "./Runtime"; import { buildGuardedCommand, buildShellPathExport } from "./shellEnv"; @@ -100,6 +101,10 @@ export function containerLabel(workspacePath: string): string { } export class DevcontainerRuntime extends LocalBaseRuntime { + override get location(): RuntimeLocation { + return "devcontainer"; + } + private readonly worktreeManager: WorktreeManager; private readonly configPath: string; /** In-container stops started by aborts and timeouts and not finished yet (#6923). */ diff --git a/src/node/runtime/LocalBaseRuntime.ts b/src/node/runtime/LocalBaseRuntime.ts index a64f86e1100..ec819b725f4 100644 --- a/src/node/runtime/LocalBaseRuntime.ts +++ b/src/node/runtime/LocalBaseRuntime.ts @@ -19,6 +19,7 @@ import type { InitLogger, EnsureReadyResult, ReadFileOptions, + RuntimeLocation, } from "./Runtime"; import { RuntimeError as RuntimeErrorClass } from "./Runtime"; import { NON_INTERACTIVE_ENV_VARS } from "@/common/constants/env"; @@ -214,6 +215,11 @@ export function localRuntimeTempRoot(): string { * - forkWorkspace() */ export abstract class LocalBaseRuntime implements Runtime { + /** Runtime.location. A prototype getter, so subclasses and Object.create doubles inherit it. */ + get location(): RuntimeLocation { + return "local"; + } + /** * The config path of the project whose agent resource limit binds this runtime's commands, or * undefined when it is not known; those commands get only the limit that all projects share. diff --git a/src/node/runtime/RemoteRuntime.ts b/src/node/runtime/RemoteRuntime.ts index cc781af70d9..3f292b94f7c 100644 --- a/src/node/runtime/RemoteRuntime.ts +++ b/src/node/runtime/RemoteRuntime.ts @@ -30,6 +30,7 @@ import type { WorkspaceForkResult, EnsureReadyResult, ReadFileOptions, + RuntimeLocation, } from "./Runtime"; import { RuntimeError, isRuntimeTransportError } from "./Runtime"; import { LEGACY_REMOTE_MUX_HOME } from "@/common/compat/legacyMux"; @@ -83,6 +84,11 @@ export interface SpawnResult { * Abstract base class for remote execution runtimes. */ export abstract class RemoteRuntime implements Runtime { + /** Runtime.location. A prototype getter, so subclasses and Object.create doubles inherit it. */ + get location(): RuntimeLocation { + return "remote"; + } + /** Remote stops started by aborts and not finished yet (#6923). */ private readonly pendingStops = new PendingStops(); diff --git a/src/node/runtime/Runtime.ts b/src/node/runtime/Runtime.ts index d41fcdbe9b8..38b59b7937f 100644 --- a/src/node/runtime/Runtime.ts +++ b/src/node/runtime/Runtime.ts @@ -462,6 +462,19 @@ export type EnsureReadyResult = */ export const WORKSPACE_REPO_MISSING_ERROR = "Workspace setup incomplete: repository not found."; +/** See Runtime.location. */ +export type RuntimeLocation = "local" | "devcontainer" | "remote"; + +/** Same answer as `runtime instanceof LocalBaseRuntime` (local, worktree, devcontainer). */ +export function isLocalBaseRuntime(runtime: Runtime | undefined): boolean { + return runtime?.location === "local" || runtime?.location === "devcontainer"; +} + +/** Same answer as `runtime instanceof RemoteRuntime` (SSH, Coder SSH, Docker). */ +export function isRemoteRuntime(runtime: Runtime | undefined): boolean { + return runtime?.location === "remote"; +} + /** * Runtime interface - minimal, low-level abstraction for tool execution environments. * @@ -474,6 +487,14 @@ export interface Runtime { * If not provided, defaults to standard behavior (no flags set). */ readonly createFlags?: RuntimeCreateFlags; + /** + * The runtime family, for code that branches on it without importing a runtime class + * (agent-loop files may not: local/no-host-imports). LocalBaseRuntime gives "local", + * DevcontainerRuntime (a LocalBaseRuntime) gives "devcontainer", and RemoteRuntime gives + * "remote", as prototype getters that subclasses and Object.create doubles inherit. + * MultiProjectRuntime and object doubles leave it absent, as instanceof is false for them. + */ + readonly location?: RuntimeLocation; /** * Execute a bash command with streaming I/O. * cwd and pathEnv values are host-namespace paths translated by the adapter. diff --git a/src/node/runtime/runtimeLocation.test.ts b/src/node/runtime/runtimeLocation.test.ts new file mode 100644 index 00000000000..dbcb1ca1982 --- /dev/null +++ b/src/node/runtime/runtimeLocation.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, test } from "bun:test"; +import { CoderSSHRuntime } from "./CoderSSHRuntime"; +import { DevcontainerRuntime } from "./DevcontainerRuntime"; +import { DockerRuntime } from "./DockerRuntime"; +import { LocalBaseRuntime } from "./LocalBaseRuntime"; +import { LocalRuntime } from "./LocalRuntime"; +import { MultiProjectRuntime } from "./multiProjectRuntime"; +import { RemoteRuntime } from "./RemoteRuntime"; +import { isLocalBaseRuntime, isRemoteRuntime, type Runtime } from "./Runtime"; +import { SSHRuntime } from "./SSHRuntime"; +import { ShellRemoteRuntime, TestRemoteRuntime } from "./testRemoteRuntime"; +import { WorktreeRuntime } from "./WorktreeRuntime"; +import { ExecPathMappingRuntime } from "@/node/services/testExecPathMappingRuntime"; +import { + RemotePathMappedRuntime, + TrueRemotePathMappedRuntime, +} from "@/node/services/tools/testHelpers"; + +// Agent-loop files branch on Runtime.location instead of instanceof (local/no-host-imports), so +// location must give today's instanceof answer for every runtime class. Object.create builds an +// instance without a constructor (no worktree, container or SSH host needed) and still reads the +// prototype getters, as the Object.create doubles in tests do. +const classes: Array<[string, abstract new (...args: never[]) => Runtime]> = [ + ["LocalRuntime", LocalRuntime], + ["WorktreeRuntime", WorktreeRuntime], + ["DevcontainerRuntime", DevcontainerRuntime], + ["SSHRuntime", SSHRuntime], + ["CoderSSHRuntime", CoderSSHRuntime], + ["DockerRuntime", DockerRuntime], + ["TestRemoteRuntime", TestRemoteRuntime], + ["ShellRemoteRuntime", ShellRemoteRuntime], + ["ExecPathMappingRuntime", ExecPathMappingRuntime], + ["MultiProjectRuntime", MultiProjectRuntime], + ["RemotePathMappedRuntime", RemotePathMappedRuntime], + ["TrueRemotePathMappedRuntime", TrueRemotePathMappedRuntime], +]; + +describe("Runtime.location", () => { + test.each(classes)("%s gives the instanceof answers", (_name, RuntimeClass) => { + const runtime = Object.create(RuntimeClass.prototype as object) as Runtime; + expect({ + local: isLocalBaseRuntime(runtime), + devcontainer: runtime.location === "devcontainer", + remote: isRemoteRuntime(runtime), + }).toEqual({ + local: runtime instanceof LocalBaseRuntime, + devcontainer: runtime instanceof DevcontainerRuntime, + remote: runtime instanceof RemoteRuntime, + }); + }); + + test("a runtime without location takes neither branch", () => { + expect([isLocalBaseRuntime(undefined), isRemoteRuntime(undefined)]).toEqual([false, false]); + }); +}); diff --git a/src/node/services/tools/agent_skill_read_file.ts b/src/node/services/tools/agent_skill_read_file.ts index 925e8896a06..1d54e3483ff 100644 --- a/src/node/services/tools/agent_skill_read_file.ts +++ b/src/node/services/tools/agent_skill_read_file.ts @@ -19,8 +19,7 @@ import { } from "@/node/services/agentSkills/skillStorageContext"; import { MAX_FILE_SIZE, validateFileSize } from "@/node/services/tools/fileCommon"; import { readBuiltInSkillFile } from "@/node/services/agentSkills/builtInSkillDefinitions"; -import { RemoteRuntime } from "@/node/runtime/RemoteRuntime"; -import { RuntimeError, type Runtime } from "@/node/runtime/Runtime"; +import { RuntimeError, isRemoteRuntime, type Runtime } from "@/node/runtime/Runtime"; import { readFileString } from "@/node/utils/runtime/helpers"; import { resolveContainedSkillFilePath } from "./skillFileUtils"; import { resolveContainedSkillFilePathOnRuntime } from "./runtimeSkillPathUtils"; @@ -148,7 +147,7 @@ export async function resolveSkillFileTarget( filePath: string ): Promise { const shouldUseRuntimeContainment = - skillRuntime instanceof RemoteRuntime || + isRemoteRuntime(skillRuntime) || (resolvedSkill.package.scope === "project" && containment.kind === "runtime"); const { resolvedPath } = shouldUseRuntimeContainment ? await resolveContainedSkillFilePathOnRuntime(skillRuntime, resolvedSkill.skillDir, filePath) diff --git a/src/node/services/tools/bash.ts b/src/node/services/tools/bash.ts index 96c38b66dc0..477abb0298b 100644 --- a/src/node/services/tools/bash.ts +++ b/src/node/services/tools/bash.ts @@ -24,8 +24,8 @@ import { TOOL_DEFINITIONS } from "@/common/utils/tools/toolDefinitions"; import { toBashTaskId } from "./taskId"; import { migrateToBackground } from "@/node/services/backgroundProcessExecutor"; import { raceWithAbortAndTimeout } from "@/node/utils/concurrency/withTimeout"; -import { LocalBaseRuntime } from "@/node/runtime/LocalBaseRuntime"; import { getToolEnvPath } from "@/node/services/hooks"; +import { isLocalBaseRuntime } from "@/node/runtime/Runtime"; import { combineGitNoRepoAutomationEnvs, gitHooksAllowed, @@ -1037,7 +1037,7 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { : [config.cwd]; const repoEnvs: Array> = []; for (const repoPath of repoPaths) { - if (config.runtime instanceof LocalBaseRuntime) { + if (isLocalBaseRuntime(config.runtime)) { await assertLocalRepoDirectoryExists(repoPath); repoEnvs.push(await gitNoRepoAutomationEnvForLocalRepo(repoPath, abortSignal, true)); } else { @@ -1060,7 +1060,7 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { // Since the bash tool runs via bash, `nul` becomes a real file in the workspace. // Only rewrite for local Windows runtimes so non-Windows scripts keep their semantics. const shouldRewriteNullRedirects = - process.platform === "win32" && config.runtime instanceof LocalBaseRuntime; + process.platform === "win32" && isLocalBaseRuntime(config.runtime); const nulRedirectRewrite = shouldRewriteNullRedirects ? rewriteWindowsNullRedirects(script) : { script, didRewrite: false }; diff --git a/src/node/services/tools/skillFileHint.ts b/src/node/services/tools/skillFileHint.ts index 694f8e692cc..64250b3b163 100644 --- a/src/node/services/tools/skillFileHint.ts +++ b/src/node/services/tools/skillFileHint.ts @@ -4,8 +4,7 @@ import type { InvocableSkill, SkillLookupName } from "@/common/types/agentSkill" import assert from "@/common/utils/assert"; import { parseSkillLookupName } from "@/common/utils/agentSkills/skillLookupName"; import type { ToolConfiguration } from "@/common/utils/tools/tools"; -import { RemoteRuntime } from "@/node/runtime/RemoteRuntime"; -import type { Runtime } from "@/node/runtime/Runtime"; +import { isRemoteRuntime, type Runtime } from "@/node/runtime/Runtime"; import { readAgentSkill } from "@/node/services/agentSkills/agentSkillsService"; import { resolveSkillDirectory } from "@/node/services/agentSkills/skillDirectory"; import { resolveSkillStorageContext } from "@/node/services/agentSkills/skillStorageContext"; @@ -101,7 +100,7 @@ async function findSkillFileForPath( filePath: string ): Promise { if (!config.cwd) return null; - const pathApi = config.runtime instanceof RemoteRuntime ? path.posix : path; + const pathApi = isRemoteRuntime(config.runtime) ? path.posix : path; // Lexical: `..` segments are resolved first, so `/../x` is outside. // Symlinks are checked by agent_skill_read_file's containment in skillOwnsFile. // pathApi.normalize: remote normalizePath keeps `..` segments. From 5177d3392543963a2465ae65ddc60559ad6c79cd Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 22 Oct 2026 22:20:44 +0000 Subject: [PATCH 4/8] lint: Runtime.location stays a prototype getter for Object.create doubles --- src/node/runtime/LocalBaseRuntime.ts | 1 + src/node/runtime/RemoteRuntime.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/src/node/runtime/LocalBaseRuntime.ts b/src/node/runtime/LocalBaseRuntime.ts index ec819b725f4..234c1fdda93 100644 --- a/src/node/runtime/LocalBaseRuntime.ts +++ b/src/node/runtime/LocalBaseRuntime.ts @@ -216,6 +216,7 @@ export function localRuntimeTempRoot(): string { */ export abstract class LocalBaseRuntime implements Runtime { /** Runtime.location. A prototype getter, so subclasses and Object.create doubles inherit it. */ + // eslint-disable-next-line @typescript-eslint/class-literal-property-style -- a readonly field is set by the constructor, so Object.create(X.prototype) doubles (mcpServerManager.test.ts) would read undefined get location(): RuntimeLocation { return "local"; } diff --git a/src/node/runtime/RemoteRuntime.ts b/src/node/runtime/RemoteRuntime.ts index 3f292b94f7c..a2583e5a5aa 100644 --- a/src/node/runtime/RemoteRuntime.ts +++ b/src/node/runtime/RemoteRuntime.ts @@ -85,6 +85,7 @@ export interface SpawnResult { */ export abstract class RemoteRuntime implements Runtime { /** Runtime.location. A prototype getter, so subclasses and Object.create doubles inherit it. */ + // eslint-disable-next-line @typescript-eslint/class-literal-property-style -- a readonly field is set by the constructor, so Object.create(X.prototype) doubles (mcpServerManager.test.ts) would read undefined get location(): RuntimeLocation { return "remote"; } From 3aaf94179f60c05a5635f4e22e4007428fd69574 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 22 Oct 2026 22:25:28 +0000 Subject: [PATCH 5/8] refactor: notify shows OS notifications through a DesktopNotifications service --- .oxlintrc.json | 1 - eslint.config.mjs | 1 - scripts/noHostImports.test.ts | 1 - src/common/utils/tools/tools.ts | 3 + src/node/services/aiService.ts | 6 +- .../desktopNotifications.ts | 34 ++++++ .../electronDesktopNotifications.test.ts | 11 ++ .../electronDesktopNotifications.ts | 88 +++++++++++++++ ...cordingDesktopNotifications.testHarness.ts | 20 ++++ src/node/services/di/layers/core.ts | 4 +- src/node/services/di/layers/stores.ts | 5 +- src/node/services/di/tags.ts | 14 ++- src/node/services/tools/notify.test.ts | 38 +++++++ src/node/services/tools/notify.ts | 106 +++--------------- src/node/services/turnRequestBuilder.ts | 12 ++ 15 files changed, 243 insertions(+), 101 deletions(-) create mode 100644 src/node/services/desktopNotifications/desktopNotifications.ts create mode 100644 src/node/services/desktopNotifications/electronDesktopNotifications.test.ts create mode 100644 src/node/services/desktopNotifications/electronDesktopNotifications.ts create mode 100644 src/node/services/desktopNotifications/recordingDesktopNotifications.testHarness.ts diff --git a/.oxlintrc.json b/.oxlintrc.json index f78745a5a72..0f5c58b9ca0 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -1269,7 +1269,6 @@ "src/node/services/tools/agent_skill_list.ts": ["fs/promises", "os"], "src/node/services/tools/agent_skill_write.ts": ["fs/promises"], "src/node/services/tools/bash.ts": ["fs"], - "src/node/services/tools/notify.ts": ["electron"], "src/node/services/tools/review_pane.ts": [ "@/node/utils/writeFileAtomic", "fs/promises" diff --git a/eslint.config.mjs b/eslint.config.mjs index 439950dc3e2..72bd6339ed3 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -138,7 +138,6 @@ const HOST_IMPORT_ALLOW = { "src/node/services/tools/agent_skill_list.ts": ["fs/promises", "os"], "src/node/services/tools/agent_skill_write.ts": ["fs/promises"], "src/node/services/tools/bash.ts": ["fs"], - "src/node/services/tools/notify.ts": ["electron"], "src/node/services/tools/review_pane.ts": ["@/node/utils/writeFileAtomic", "fs/promises"], "src/node/services/tools/shared/configReadWrite.ts": [ "@/node/utils/writeFileAtomic", diff --git a/scripts/noHostImports.test.ts b/scripts/noHostImports.test.ts index a3701388fb0..5db80b4fefc 100644 --- a/scripts/noHostImports.test.ts +++ b/scripts/noHostImports.test.ts @@ -133,7 +133,6 @@ const FROZEN_ALLOW_PAIRS = [ "src/node/services/tools/agent_skill_list.ts os", "src/node/services/tools/agent_skill_write.ts fs/promises", "src/node/services/tools/bash.ts fs", - "src/node/services/tools/notify.ts electron", "src/node/services/tools/review_pane.ts @/node/utils/writeFileAtomic", "src/node/services/tools/review_pane.ts fs/promises", "src/node/services/tools/shared/configReadWrite.ts @/node/utils/writeFileAtomic", diff --git a/src/common/utils/tools/tools.ts b/src/common/utils/tools/tools.ts index ccc70815f33..ac97bcfd865 100644 --- a/src/common/utils/tools/tools.ts +++ b/src/common/utils/tools/tools.ts @@ -91,6 +91,7 @@ import * as os from "os"; import type { InitStateManager } from "@/node/services/initStateManager"; import type { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import type { DesktopSessionManager } from "@/node/services/desktop/DesktopSessionManager"; +import type { DesktopNotifications } from "@/node/services/desktopNotifications/desktopNotifications"; import type { ComputerUseService } from "@/node/services/computerUse/computerUseService"; import type { TaskService } from "@/node/services/taskService"; import type { WorkspaceTurnManager } from "@/node/services/workspaceTurnManager"; @@ -162,6 +163,8 @@ export interface ToolConfiguration { hookTurn?: HookTurn; /** Runtime environment for executing commands and file operations */ runtime: Runtime; + /** OS notifications for the notify tool. Absent: the Node layer (configs that tests build by hand). */ + desktopNotifications?: DesktopNotifications; /** Project roots in this workspace (single- or multi-project context for tool descriptions) */ projects?: ProjectRef[]; /** Environment secrets to inject (optional) */ diff --git a/src/node/services/aiService.ts b/src/node/services/aiService.ts index 971a2604171..6bcfd1af053 100644 --- a/src/node/services/aiService.ts +++ b/src/node/services/aiService.ts @@ -27,6 +27,7 @@ import type { WorkspaceMetadata } from "@/common/types/workspace"; import { ensurePrivateDir } from "@/node/utils/fs"; import { TurnRequestBuilder, + type LoopHostServices, type TurnRequestBuilderBindings, resolveXumToolScope, type StreamMessageOptions, @@ -215,7 +216,9 @@ export class AIService extends EventEmitter { providersConfigStore?: ProvidersConfigStore, private readonly secretsStore: Pick = new SecretsStore( config.rootDir - ) + ), + /** Absent members: the Node layer (`new AIService(` in tests passes none). */ + hostServices: LoopHostServices = {} ) { super(); // No listener cap: every live AgentSession adds one listener per forwarded event, and a @@ -268,6 +271,7 @@ export class AIService extends EventEmitter { experimentsService: this.experimentsService, lastLlmRequestByWorkspace: this.lastLlmRequestByWorkspace, bindings: this.turnRequestBuilderBindings, + hostServices, emit: (event, ...args) => this.emit(event, ...args), createAbortedTurnHandle: (messageId, signal) => this.createAbortedTurnHandle(messageId, signal), diff --git a/src/node/services/desktopNotifications/desktopNotifications.ts b/src/node/services/desktopNotifications/desktopNotifications.ts new file mode 100644 index 00000000000..9b275166b90 --- /dev/null +++ b/src/node/services/desktopNotifications/desktopNotifications.ts @@ -0,0 +1,34 @@ +/** + * The OS notification that the notify tool shows. Loop code (notify.ts) reads this interface + * from ToolConfiguration instead of importing electron (local/no-host-imports). The Node layer + * is electronDesktopNotifications; tests pass a recording one + * (recordingDesktopNotifications.testHarness.ts). + */ + +/** Maximum notification body length (macOS limit is 256 bytes) */ +export const MAX_NOTIFICATION_BODY_LENGTH = 200; + +/** Maximum notification title length */ +export const MAX_NOTIFICATION_TITLE_LENGTH = 64; + +/** + * Truncates text to a maximum length, adding ellipsis if truncated + */ +export function truncateText(text: string, maxLength: number): string { + if (text.length <= maxLength) { + return text; + } + return text.slice(0, maxLength - 1) + "…"; +} + +export interface DesktopNotificationRequest { + title: string; + body?: string; + /** Clicking the notification navigates to this workspace. */ + workspaceId?: string; +} + +export interface DesktopNotifications { + /** success false: no OS notification; the notify tool then falls back to the browser. */ + show(request: DesktopNotificationRequest): Promise<{ success: boolean; error?: string }>; +} diff --git a/src/node/services/desktopNotifications/electronDesktopNotifications.test.ts b/src/node/services/desktopNotifications/electronDesktopNotifications.test.ts new file mode 100644 index 00000000000..d4f9b070645 --- /dev/null +++ b/src/node/services/desktopNotifications/electronDesktopNotifications.test.ts @@ -0,0 +1,11 @@ +import { expect, test } from "bun:test"; +import { electronDesktopNotifications } from "./electronDesktopNotifications"; + +// bun test runs outside Electron: the Node layer must refuse before it imports electron, with +// the error that notify.ts returned before the move, so the tool still falls back to the browser. +test("outside Electron the Node layer shows nothing and says why", async () => { + expect(await electronDesktopNotifications.show({ title: "t", workspaceId: "ws" })).toEqual({ + success: false, + error: "System notifications not supported (not running in Electron)", + }); +}); diff --git a/src/node/services/desktopNotifications/electronDesktopNotifications.ts b/src/node/services/desktopNotifications/electronDesktopNotifications.ts new file mode 100644 index 00000000000..524007a8988 --- /dev/null +++ b/src/node/services/desktopNotifications/electronDesktopNotifications.ts @@ -0,0 +1,88 @@ +/** + * The Node layer of DesktopNotifications: Electron's Notification API, moved here from + * tools/notify.ts so that loop code does not import electron (local/no-host-imports). + * This file is not an agent-loop file. + */ + +import { getErrorMessage } from "@/common/utils/errors"; +import { + MAX_NOTIFICATION_BODY_LENGTH, + MAX_NOTIFICATION_TITLE_LENGTH, + truncateText, + type DesktopNotifications, +} from "./desktopNotifications"; + +/** + * Check if we're running in Electron environment + */ +function isElectronEnvironment(): boolean { + return typeof process.versions.electron === "string"; +} + +/** + * Send a system notification using Electron's Notification API. + * When clicked, focuses the app and navigates to the specified workspace. + * Returns true if notification was shown, false otherwise. + */ +async function sendElectronNotification( + title: string, + body?: string, + workspaceId?: string +): Promise<{ success: boolean; error?: string }> { + if (!isElectronEnvironment()) { + return { + success: false, + error: "System notifications not supported (not running in Electron)", + }; + } + + try { + // eslint-disable-next-line no-restricted-syntax -- Electron is unavailable in `mux server`; avoid top-level import + const { Notification, BrowserWindow } = await import("electron"); + + if (!Notification.isSupported()) { + return { + success: false, + error: "System notifications not supported on this platform", + }; + } + + const notification = new Notification({ + title: truncateText(title, MAX_NOTIFICATION_TITLE_LENGTH), + body: body ? truncateText(body, MAX_NOTIFICATION_BODY_LENGTH) : undefined, + silent: false, + }); + + // Handle notification click - focus app and navigate to workspace + notification.on("click", () => { + const windows = BrowserWindow.getAllWindows(); + const mainWindow = windows[0]; + if (mainWindow) { + // Restore if minimized, then focus + if (mainWindow.isMinimized()) { + mainWindow.restore(); + } + mainWindow.focus(); + + // Send IPC message to renderer to navigate to workspace + if (workspaceId) { + mainWindow.webContents.send("mux:notification-clicked", { workspaceId }); + } + } + }); + + notification.show(); + + return { success: true }; + } catch (error) { + const message = getErrorMessage(error); + return { + success: false, + error: `Failed to send notification: ${message}`, + }; + } +} + +export const electronDesktopNotifications: DesktopNotifications = { + show: (request) => sendElectronNotification(request.title, request.body, request.workspaceId), +}; diff --git a/src/node/services/desktopNotifications/recordingDesktopNotifications.testHarness.ts b/src/node/services/desktopNotifications/recordingDesktopNotifications.testHarness.ts new file mode 100644 index 00000000000..d8d16d1c2c5 --- /dev/null +++ b/src/node/services/desktopNotifications/recordingDesktopNotifications.testHarness.ts @@ -0,0 +1,20 @@ +import type { DesktopNotificationRequest, DesktopNotifications } from "./desktopNotifications"; + +/** The test layer of DesktopNotifications: it shows nothing and records each request. */ +export interface RecordingDesktopNotifications extends DesktopNotifications { + readonly requests: DesktopNotificationRequest[]; +} + +/** result: what show() answers, as an OS notification would (default: shown). */ +export function createRecordingDesktopNotifications( + result: { success: boolean; error?: string } = { success: true } +): RecordingDesktopNotifications { + const requests: DesktopNotificationRequest[] = []; + return { + requests, + show: (request) => { + requests.push(request); + return Promise.resolve(result); + }, + }; +} diff --git a/src/node/services/di/layers/core.ts b/src/node/services/di/layers/core.ts index 52f156920c1..4e973a727cf 100644 --- a/src/node/services/di/layers/core.ts +++ b/src/node/services/di/layers/core.ts @@ -32,6 +32,7 @@ import { BackgroundProcessManagerTag, ConfigTag, ContextManagement, + DesktopNotificationsTag, Evaluation, ExtensionMetadata, FileLeaseManagerTag, @@ -328,7 +329,8 @@ export const AILive = Layer.effect( yield* StreamManagerTag, yield* TurnRequestBuilderBindingsTag, yield* ProvidersConfigStoreTag, - yield* SecretsStoreTag + yield* SecretsStoreTag, + { desktopNotifications: yield* DesktopNotificationsTag } ); }) ); diff --git a/src/node/services/di/layers/stores.ts b/src/node/services/di/layers/stores.ts index a42756e4123..8c57cd9bdfc 100644 --- a/src/node/services/di/layers/stores.ts +++ b/src/node/services/di/layers/stores.ts @@ -7,8 +7,10 @@ import { type ConfigStores, } from "@/node/config"; import type { CoreServicesOptions } from "@/node/services/coreServices"; +import { electronDesktopNotifications } from "@/node/services/desktopNotifications/electronDesktopNotifications"; import { ConfigTag, + DesktopNotificationsTag, FileLeaseManagerTag, ProvidersConfigStoreTag, SecretsStoreTag, @@ -28,7 +30,8 @@ export function StoresLive(stores: ConfigStores): Layer.Layer { Layer.succeed(SessionLocatorTag)(stores.sessionLocator), Layer.succeed(ProvidersConfigStoreTag)(stores.providersConfigStore), Layer.succeed(SecretsStoreTag)(stores.secretsStore), - Layer.succeed(FileLeaseManagerTag)(stores.fileLeaseManager) + Layer.succeed(FileLeaseManagerTag)(stores.fileLeaseManager), + Layer.succeed(DesktopNotificationsTag)(electronDesktopNotifications) ); } diff --git a/src/node/services/di/tags.ts b/src/node/services/di/tags.ts index f9b473166b7..1e88679ad7c 100644 --- a/src/node/services/di/tags.ts +++ b/src/node/services/di/tags.ts @@ -27,6 +27,7 @@ import type { AutoModelRouter } from "@/node/services/autoModelRouter"; import type { AnalyticsService } from "@/node/services/analytics/analyticsService"; import type { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import type { BackupService } from "@/node/services/backup/backupService"; +import type { DesktopNotifications } from "@/node/services/desktopNotifications/desktopNotifications"; import type { AgentBrowserSessionDiscoveryService } from "@/node/services/browser/AgentBrowserSessionDiscoveryService"; import type { BrowserBridgeServer } from "@/node/services/browser/BrowserBridgeServer"; import type { BrowserBridgeTokenManager } from "@/node/services/browser/BrowserBridgeTokenManager"; @@ -106,6 +107,11 @@ export class ProvidersConfigStoreTag extends Context.Service< export class SecretsStoreTag extends Context.Service()( "xum/SecretsStore" ) {} +/** OS notifications of the notify tool (desktopNotifications/). */ +export class DesktopNotificationsTag extends Context.Service< + DesktopNotificationsTag, + DesktopNotifications +>()("xum/DesktopNotifications") {} export class FileLeaseManagerTag extends Context.Service()( "xum/FileLeaseManager" ) {} @@ -312,13 +318,17 @@ export class BashAiProxy extends Context.Service { let config: ToolConfiguration; @@ -120,4 +121,41 @@ describe("notify tool", () => { expect(result.ui_only?.notify?.workspaceId).toBe("test-workspace-123"); } }); + + it("shows the notification through config.desktopNotifications", async () => { + const notifications = createRecordingDesktopNotifications(); + const tool = createNotifyTool({ + ...config, + workspaceId: "ws-1", + desktopNotifications: notifications, + }); + const execute = tool.execute as (args: { + title: string; + message?: string; + }) => Promise; + + const result = await execute({ title: ` ${"t".repeat(70)} `, message: " body " }); + + const title = `${"t".repeat(63)}…`; + expect(notifications.requests).toEqual([{ title, body: "body", workspaceId: "ws-1" }]); + expect(result).toMatchObject({ + success: true, + title, + ui_only: { notify: { notifiedVia: "electron", workspaceId: "ws-1" } }, + }); + }); + + it("falls back to the browser when desktopNotifications shows nothing", async () => { + const notifications = createRecordingDesktopNotifications({ success: false, error: "no" }); + const tool = createNotifyTool({ ...config, desktopNotifications: notifications }); + const execute = tool.execute as (args: { title: string }) => Promise; + + const result = await execute({ title: "Done" }); + + expect(notifications.requests).toHaveLength(1); + expect(result).toMatchObject({ + success: true, + ui_only: { notify: { notifiedVia: "browser" } }, + }); + }); }); diff --git a/src/node/services/tools/notify.ts b/src/node/services/tools/notify.ts index 20fa988172d..d8a45b35b34 100644 --- a/src/node/services/tools/notify.ts +++ b/src/node/services/tools/notify.ts @@ -14,94 +14,12 @@ import { tool } from "ai"; import type { ToolFactory } from "@/common/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/common/utils/tools/toolDefinitions"; import type { NotifyToolResult } from "@/common/types/tools"; -import { getErrorMessage } from "@/common/utils/errors"; - -/** Maximum notification body length (macOS limit is 256 bytes) */ -const MAX_NOTIFICATION_BODY_LENGTH = 200; - -/** Maximum notification title length */ -const MAX_NOTIFICATION_TITLE_LENGTH = 64; - -/** - * Truncates text to a maximum length, adding ellipsis if truncated - */ -function truncateText(text: string, maxLength: number): string { - if (text.length <= maxLength) { - return text; - } - return text.slice(0, maxLength - 1) + "…"; -} - -/** - * Check if we're running in Electron environment - */ -function isElectronEnvironment(): boolean { - return typeof process.versions.electron === "string"; -} - -/** - * Send a system notification using Electron's Notification API. - * When clicked, focuses the app and navigates to the specified workspace. - * Returns true if notification was shown, false otherwise. - */ -async function sendElectronNotification( - title: string, - body?: string, - workspaceId?: string -): Promise<{ success: boolean; error?: string }> { - if (!isElectronEnvironment()) { - return { - success: false, - error: "System notifications not supported (not running in Electron)", - }; - } - - try { - // eslint-disable-next-line no-restricted-syntax -- Electron is unavailable in `mux server`; avoid top-level import - const { Notification, BrowserWindow } = await import("electron"); - - if (!Notification.isSupported()) { - return { - success: false, - error: "System notifications not supported on this platform", - }; - } - - const notification = new Notification({ - title: truncateText(title, MAX_NOTIFICATION_TITLE_LENGTH), - body: body ? truncateText(body, MAX_NOTIFICATION_BODY_LENGTH) : undefined, - silent: false, - }); - - // Handle notification click - focus app and navigate to workspace - notification.on("click", () => { - const windows = BrowserWindow.getAllWindows(); - const mainWindow = windows[0]; - if (mainWindow) { - // Restore if minimized, then focus - if (mainWindow.isMinimized()) { - mainWindow.restore(); - } - mainWindow.focus(); - - // Send IPC message to renderer to navigate to workspace - if (workspaceId) { - mainWindow.webContents.send("mux:notification-clicked", { workspaceId }); - } - } - }); - - notification.show(); - - return { success: true }; - } catch (error) { - const message = getErrorMessage(error); - return { - success: false, - error: `Failed to send notification: ${message}`, - }; - } -} +import { + MAX_NOTIFICATION_BODY_LENGTH, + MAX_NOTIFICATION_TITLE_LENGTH, + truncateText, +} from "@/node/services/desktopNotifications/desktopNotifications"; +import { electronDesktopNotifications } from "@/node/services/desktopNotifications/electronDesktopNotifications"; /** * Notify tool factory for AI assistant @@ -128,11 +46,13 @@ export const createNotifyTool: ToolFactory = (config) => { ? truncateText(trimmedMessage, MAX_NOTIFICATION_BODY_LENGTH) : undefined; - const result = await sendElectronNotification( - truncatedTitle, - truncatedMessage, - config.workspaceId - ); + // Absent: the Node layer, for configs that tests build by hand. + const notifications = config.desktopNotifications ?? electronDesktopNotifications; + const result = await notifications.show({ + title: truncatedTitle, + body: truncatedMessage, + workspaceId: config.workspaceId, + }); if (config.workspaceId != null && config.timelineService != null) { config.timelineService.record(config.workspaceId, { diff --git a/src/node/services/turnRequestBuilder.ts b/src/node/services/turnRequestBuilder.ts index 4891fee6697..e6ae4cbe406 100644 --- a/src/node/services/turnRequestBuilder.ts +++ b/src/node/services/turnRequestBuilder.ts @@ -105,6 +105,7 @@ import { DEFAULT_ROUTE_PRIORITY } from "@/common/routing"; import { createDisplayUsage } from "@/common/utils/tokens/displayUsage"; import { getTotalCost, sumUsageHistory } from "@/common/utils/tokens/usageAggregator"; import type { DesktopSessionManager } from "@/node/services/desktop/DesktopSessionManager"; +import type { DesktopNotifications } from "@/node/services/desktopNotifications/desktopNotifications"; import type { BashAiProxyService } from "@/node/services/bashAiProxy/bashAiProxyService"; import type { ComputerUseService } from "@/node/services/computerUse/computerUseService"; import type { DevToolsService } from "@/node/services/devToolsService"; @@ -947,6 +948,15 @@ function requireWorkflowArchiveAdmission( return guard; } +/** + * Host services that tools read through ToolConfiguration instead of importing host modules + * (local/no-host-imports). AILive passes the Node layers; an absent member means the tool's + * own Node default. + */ +export interface LoopHostServices { + desktopNotifications?: DesktopNotifications; +} + interface TurnRequestBuilderDependencies { config: Config; providersConfigStore: ProvidersConfigStore; @@ -967,6 +977,7 @@ interface TurnRequestBuilderDependencies { set(workspaceId: string, snapshot: DebugLlmRequestSnapshot): unknown; }; bindings: TurnRequestBuilderBindings; + hostServices?: LoopHostServices; emit: (event: string, ...args: unknown[]) => boolean; createAbortedTurnHandle: (messageId: string, signal?: AbortSignal) => TurnStreamHandle; createSettledTurnHandle: (messageId: string, completion: TurnCompletion) => TurnStreamHandle; @@ -2884,6 +2895,7 @@ export class TurnRequestBuilder { strictAgentResolution, xumScope, timelineService: this.dependencies.bindings.timelineService, + desktopNotifications: this.dependencies.hostServices?.desktopNotifications, // Agent-scheduled heartbeats start paid turns unattended, so they are user opt-in. workspaceHeartbeatService: this.dependencies.config.loadConfigOrDefault().agentHeartbeatsEnabled === true From 9bbf9ea23455fa0d0690ed44ccca0aa990b1279f Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 22 Oct 2026 22:25:57 +0000 Subject: [PATCH 6/8] refactor: TurnRequestBuilder always gets the loop host services --- src/node/services/turnRequestBuilder.test.ts | 1 + src/node/services/turnRequestBuilder.ts | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/node/services/turnRequestBuilder.test.ts b/src/node/services/turnRequestBuilder.test.ts index c29693d9800..196f8310daf 100644 --- a/src/node/services/turnRequestBuilder.test.ts +++ b/src/node/services/turnRequestBuilder.test.ts @@ -52,6 +52,7 @@ async function createPreparationHarness() { workspaceMcpOverridesService: new WorkspaceMcpOverridesService(config), lastLlmRequestByWorkspace: new Map(), bindings: {}, + hostServices: {}, emit: () => false, createAbortedTurnHandle: (messageId) => ({ messageId, diff --git a/src/node/services/turnRequestBuilder.ts b/src/node/services/turnRequestBuilder.ts index e6ae4cbe406..94a96939184 100644 --- a/src/node/services/turnRequestBuilder.ts +++ b/src/node/services/turnRequestBuilder.ts @@ -977,7 +977,7 @@ interface TurnRequestBuilderDependencies { set(workspaceId: string, snapshot: DebugLlmRequestSnapshot): unknown; }; bindings: TurnRequestBuilderBindings; - hostServices?: LoopHostServices; + hostServices: LoopHostServices; emit: (event: string, ...args: unknown[]) => boolean; createAbortedTurnHandle: (messageId: string, signal?: AbortSignal) => TurnStreamHandle; createSettledTurnHandle: (messageId: string, completion: TurnCompletion) => TurnStreamHandle; @@ -2895,7 +2895,7 @@ export class TurnRequestBuilder { strictAgentResolution, xumScope, timelineService: this.dependencies.bindings.timelineService, - desktopNotifications: this.dependencies.hostServices?.desktopNotifications, + desktopNotifications: this.dependencies.hostServices.desktopNotifications, // Agent-scheduled heartbeats start paid turns unattended, so they are user opt-in. workspaceHeartbeatService: this.dependencies.config.loadConfigOrDefault().agentHeartbeatsEnabled === true From 492720530d0f5201ec933e386aab53f05ea14247 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 23 Oct 2026 05:32:18 +0000 Subject: [PATCH 7/8] refactor: ipAddress asserts the isIP answer with the shared assert helper --- src/node/utils/ipAddress.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/node/utils/ipAddress.ts b/src/node/utils/ipAddress.ts index 889164bbf95..55bf7da69db 100644 --- a/src/node/utils/ipAddress.ts +++ b/src/node/utils/ipAddress.ts @@ -1,4 +1,5 @@ import * as net from "node:net"; +import assert from "@/common/utils/assert"; /** * The IP version of a literal address: 4, 6, or 0 when `input` is not one. @@ -9,8 +10,9 @@ import * as net from "node:net"; */ export function isIP(input: string): 0 | 4 | 6 { const version = net.isIP(input); - if (version !== 0 && version !== 4 && version !== 6) { - throw new Error(`net.isIP returned an unknown IP version: ${version}`); - } + assert( + version === 0 || version === 4 || version === 6, + `net.isIP returned an unknown IP version: ${version}` + ); return version; } From c27edf38367d324d43a26d04e9f8027d096f4dbd Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 23 Oct 2026 14:49:12 +0000 Subject: [PATCH 8/8] =?UTF-8?q?=F0=9F=A4=96=20test:=20the=20Electron=20not?= =?UTF-8?q?ification=20layer=20takes=20its=20electron=20host,=20and=20test?= =?UTF-8?q?s=20cover=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Quality / CRAP flagged sendElectronNotification as a new function (moved from notify.ts) at CRAP 34. createElectronDesktopNotifications takes the electron check and import, and tests drive a fake electron module through the shown notification, the unsupported platform, a failed import and the click. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$85.60`_ --- .../electronDesktopNotifications.test.ts | 119 +++++++++++++++++- .../electronDesktopNotifications.ts | 56 +++++++-- 2 files changed, 162 insertions(+), 13 deletions(-) diff --git a/src/node/services/desktopNotifications/electronDesktopNotifications.test.ts b/src/node/services/desktopNotifications/electronDesktopNotifications.test.ts index d4f9b070645..c81c7f42770 100644 --- a/src/node/services/desktopNotifications/electronDesktopNotifications.test.ts +++ b/src/node/services/desktopNotifications/electronDesktopNotifications.test.ts @@ -1,5 +1,13 @@ -import { expect, test } from "bun:test"; -import { electronDesktopNotifications } from "./electronDesktopNotifications"; +import { describe, expect, test } from "bun:test"; +import { + MAX_NOTIFICATION_BODY_LENGTH, + MAX_NOTIFICATION_TITLE_LENGTH, +} from "./desktopNotifications"; +import { + createElectronDesktopNotifications, + electronDesktopNotifications, + type ElectronNotificationApi, +} from "./electronDesktopNotifications"; // bun test runs outside Electron: the Node layer must refuse before it imports electron, with // the error that notify.ts returned before the move, so the tool still falls back to the browser. @@ -9,3 +17,110 @@ test("outside Electron the Node layer shows nothing and says why", async () => { error: "System notifications not supported (not running in Electron)", }); }); + +interface FakeWindow { + minimized: boolean; + calls: string[]; + sent: Array<[string, { workspaceId: string }]>; +} + +/** A fake electron module that records what the layer does with it. */ +function fakeElectron(options: { supported?: boolean; windows?: FakeWindow[] } = {}) { + const shown: Array<{ title: string; body?: string; silent: boolean }> = []; + const clickListeners: Array<() => void> = []; + const windows = options.windows ?? []; + const api: ElectronNotificationApi = { + Notification: class { + static isSupported() { + return options.supported ?? true; + } + constructor(private readonly notificationOptions: (typeof shown)[number]) {} + on(_event: "click", listener: () => void) { + clickListeners.push(listener); + } + show() { + shown.push(this.notificationOptions); + } + }, + BrowserWindow: { + getAllWindows: () => + windows.map((window) => ({ + isMinimized: () => window.minimized, + restore: () => window.calls.push("restore"), + focus: () => window.calls.push("focus"), + webContents: { send: (channel, payload) => window.sent.push([channel, payload]) }, + })), + }, + }; + const notifications = createElectronDesktopNotifications({ + isElectron: () => true, + loadElectron: () => Promise.resolve(api), + }); + return { notifications, shown, click: () => clickListeners.forEach((listener) => listener()) }; +} + +function fakeWindow(minimized: boolean): FakeWindow { + return { minimized, calls: [], sent: [] }; +} + +describe("the Node layer in Electron", () => { + test("shows one notification with the title and body cut to the OS limits", async () => { + const electron = fakeElectron(); + const result = await electron.notifications.show({ + title: "t".repeat(MAX_NOTIFICATION_TITLE_LENGTH + 5), + body: "b".repeat(MAX_NOTIFICATION_BODY_LENGTH + 5), + }); + expect(result).toEqual({ success: true }); + expect(electron.shown).toEqual([ + { + title: `${"t".repeat(MAX_NOTIFICATION_TITLE_LENGTH - 1)}…`, + body: `${"b".repeat(MAX_NOTIFICATION_BODY_LENGTH - 1)}…`, + silent: false, + }, + ]); + }); + + test("shows nothing on a platform without notifications", async () => { + const electron = fakeElectron({ supported: false }); + expect(await electron.notifications.show({ title: "t" })).toEqual({ + success: false, + error: "System notifications not supported on this platform", + }); + expect(electron.shown).toEqual([]); + }); + + test("reports a failed electron import as an error", async () => { + const notifications = createElectronDesktopNotifications({ + isElectron: () => true, + loadElectron: () => Promise.reject(new Error("no electron")), + }); + expect(await notifications.show({ title: "t" })).toEqual({ + success: false, + error: "Failed to send notification: no electron", + }); + }); + + test("a click restores and focuses the first window, then opens the workspace", async () => { + const first = fakeWindow(true); + const second = fakeWindow(false); + const electron = fakeElectron({ windows: [first, second] }); + await electron.notifications.show({ title: "t", workspaceId: "ws-1" }); + electron.click(); + expect(first.calls).toEqual(["restore", "focus"]); + expect(first.sent).toEqual([["mux:notification-clicked", { workspaceId: "ws-1" }]]); + expect(second.calls).toEqual([]); + }); + + test("a click without a workspace only focuses, and a click with no window does nothing", async () => { + const window = fakeWindow(false); + const electron = fakeElectron({ windows: [window] }); + await electron.notifications.show({ title: "t" }); + electron.click(); + expect(window.calls).toEqual(["focus"]); + expect(window.sent).toEqual([]); + + const noWindow = fakeElectron(); + await noWindow.notifications.show({ title: "t", workspaceId: "ws-1" }); + expect(() => noWindow.click()).not.toThrow(); + }); +}); diff --git a/src/node/services/desktopNotifications/electronDesktopNotifications.ts b/src/node/services/desktopNotifications/electronDesktopNotifications.ts index 524007a8988..37bbac4f3ae 100644 --- a/src/node/services/desktopNotifications/electronDesktopNotifications.ts +++ b/src/node/services/desktopNotifications/electronDesktopNotifications.ts @@ -12,24 +12,53 @@ import { type DesktopNotifications, } from "./desktopNotifications"; -/** - * Check if we're running in Electron environment - */ -function isElectronEnvironment(): boolean { - return typeof process.versions.electron === "string"; +/** The parts of the electron module that this layer uses. Tests pass a fake one. */ +export interface ElectronNotificationApi { + Notification: { + isSupported(): boolean; + new (options: { title: string; body?: string; silent: boolean }): { + on(event: "click", listener: () => void): unknown; + show(): void; + }; + }; + BrowserWindow: { + getAllWindows(): Array<{ + isMinimized(): boolean; + restore(): void; + focus(): void; + webContents: { send(channel: string, payload: { workspaceId: string }): void }; + }>; + }; } +export interface ElectronHost { + /** True when this process runs in Electron. */ + isElectron(): boolean; + /** Imports electron. Called only when isElectron() is true. */ + loadElectron(): Promise; +} + +const realElectronHost: ElectronHost = { + isElectron: () => typeof process.versions.electron === "string", + loadElectron: async () => { + // eslint-disable-next-line no-restricted-syntax -- Electron is unavailable in `mux server`; avoid top-level import + const { Notification, BrowserWindow } = await import("electron"); + return { Notification, BrowserWindow }; + }, +}; + /** * Send a system notification using Electron's Notification API. * When clicked, focuses the app and navigates to the specified workspace. * Returns true if notification was shown, false otherwise. */ async function sendElectronNotification( + host: ElectronHost, title: string, body?: string, workspaceId?: string ): Promise<{ success: boolean; error?: string }> { - if (!isElectronEnvironment()) { + if (!host.isElectron()) { return { success: false, error: "System notifications not supported (not running in Electron)", @@ -37,8 +66,7 @@ async function sendElectronNotification( } try { - // eslint-disable-next-line no-restricted-syntax -- Electron is unavailable in `mux server`; avoid top-level import - const { Notification, BrowserWindow } = await import("electron"); + const { Notification, BrowserWindow } = await host.loadElectron(); if (!Notification.isSupported()) { return { @@ -83,6 +111,12 @@ async function sendElectronNotification( } } -export const electronDesktopNotifications: DesktopNotifications = { - show: (request) => sendElectronNotification(request.title, request.body, request.workspaceId), -}; +/** The Node layer on a given electron host. Tests pass a fake host. */ +export function createElectronDesktopNotifications(host: ElectronHost): DesktopNotifications { + return { + show: (request) => + sendElectronNotification(host, request.title, request.body, request.workspaceId), + }; +} + +export const electronDesktopNotifications = createElectronDesktopNotifications(realElectronHost);