diff --git a/CHANGELOG.md b/CHANGELOG.md index fd4f0b69f1e..00623840f80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,7 @@ Docs: https://docs.openclaw.ai - Plugins/Google Meet: include live Chrome-node readiness in `googlemeet setup` and document the Parallels recovery checks, so stale node tokens or disconnected VM browsers are visible before an agent opens a meeting. Thanks @steipete. - Plugins/runtime deps: isolate the internal npm cache used for bundled plugin runtime-dependency repair and let package updates refresh/verify already-current installs, so failed update or sudo doctor runs can be repaired by rerunning `openclaw update`. Thanks @steipete. +- Agents/delete: keep `--json` output machine-readable and retain workspaces that overlap another agent's workspace instead of moving shared state to Trash. Fixes #70889 and #70890. (#70897) Thanks @kaseonedge. - Plugins/runtime deps: stage bundled plugin runtime dependencies for packaged/global installs in an external runtime root and retain already staged deps across repairs, avoiding package-tree update races and npm pruning after upgrades. Thanks @steipete. - Plugins/runtime deps: log bundled plugin runtime-dependency staging before synchronous npm installs start and include elapsed timing afterward, so first boot after upgrades no longer looks hung while dependencies are being repaired. Thanks @steipete. - Agents/failover: forward embedded run abort signals into provider-owned model streams, cap implicit LLM idle watchdogs below long run timeouts, and mark 429 responses without usable retry timing as non-retryable so GitHub Copilot rate limits fail over or surface promptly instead of hanging until run timeout. Fixes #71120. Thanks @steipete. diff --git a/docs/cli/agents.md b/docs/cli/agents.md index 1071f1a5cad..79ad81340b8 100644 --- a/docs/cli/agents.md +++ b/docs/cli/agents.md @@ -150,6 +150,9 @@ Notes: - `main` cannot be deleted. - Without `--force`, interactive confirmation is required. - Workspace, agent state, and session transcript directories are moved to Trash, not hard-deleted. +- If another agent's workspace is the same path, inside this workspace, or contains this workspace, + the workspace is retained and `--json` reports `workspaceRetained`, + `workspaceRetainedReason`, and `workspaceSharedWith`. ## Identity files diff --git a/src/auto-reply/reply/dispatch-from-config.shared.test-harness.ts b/src/auto-reply/reply/dispatch-from-config.shared.test-harness.ts index 38d1bf1e190..748397ef8a4 100644 --- a/src/auto-reply/reply/dispatch-from-config.shared.test-harness.ts +++ b/src/auto-reply/reply/dispatch-from-config.shared.test-harness.ts @@ -194,6 +194,7 @@ vi.mock("./dispatch-from-config.runtime.js", () => ({ vi.mock("../../plugins/hook-runner-global.js", () => ({ getGlobalHookRunner: () => hookMocks.runner, getGlobalPluginRegistry: () => hookMocks.registry, + initializeGlobalHookRunner: vi.fn(), })); vi.mock("../../acp/runtime/session-meta.js", () => ({ listAcpSessionEntries: acpMocks.listAcpSessionEntries, diff --git a/src/commands/agents.commands.delete.ts b/src/commands/agents.commands.delete.ts index b7a28af57fb..909168ea806 100644 --- a/src/commands/agents.commands.delete.ts +++ b/src/commands/agents.commands.delete.ts @@ -1,10 +1,14 @@ +import fs from "node:fs"; +import path from "node:path"; import { resolveAgentDir, resolveAgentWorkspaceDir } from "../agents/agent-scope.js"; import { replaceConfigFile } from "../config/config.js"; import { logConfigUpdated } from "../config/logging.js"; import { resolveSessionTranscriptsDirForAgent } from "../config/sessions.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; import { DEFAULT_AGENT_ID, normalizeAgentId } from "../routing/session-key.js"; import { type RuntimeEnv, writeRuntimeJson } from "../runtime.js"; import { defaultRuntime } from "../runtime.js"; +import { lowercasePreservingWhitespace } from "../shared/string-coerce.js"; import { createClackPrompter } from "../wizard/clack-prompter.js"; import { createQuietRuntime, @@ -14,6 +18,55 @@ import { import { findAgentEntryIndex, listAgentEntries, pruneAgentConfig } from "./agents.config.js"; import { moveToTrash } from "./onboard-helpers.js"; +function normalizeWorkspacePathForComparison(input: string): string { + const resolved = path.resolve(input.replaceAll("\0", "")); + let normalized = resolved; + try { + normalized = fs.realpathSync.native(resolved); + } catch { + // Keep lexical path for non-existent directories. + } + if (process.platform === "win32") { + return lowercasePreservingWhitespace(normalized); + } + return normalized; +} + +function isPathWithinRoot(candidatePath: string, rootPath: string): boolean { + const relative = path.relative(rootPath, candidatePath); + return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); +} + +function workspacePathsOverlap(left: string, right: string): boolean { + const normalizedLeft = normalizeWorkspacePathForComparison(left); + const normalizedRight = normalizeWorkspacePathForComparison(right); + return ( + isPathWithinRoot(normalizedLeft, normalizedRight) || + isPathWithinRoot(normalizedRight, normalizedLeft) + ); +} + +function findOverlappingWorkspaceAgentIds( + cfg: OpenClawConfig, + agentId: string, + workspaceDir: string, +): string[] { + const entries = listAgentEntries(cfg); + const normalizedAgentId = normalizeAgentId(agentId); + const overlappingAgentIds: string[] = []; + for (const entry of entries) { + const otherAgentId = normalizeAgentId(entry.id); + if (otherAgentId === normalizedAgentId) { + continue; + } + const otherWorkspace = resolveAgentWorkspaceDir(cfg, otherAgentId); + if (workspacePathsOverlap(workspaceDir, otherWorkspace)) { + overlappingAgentIds.push(otherAgentId); + } + } + return overlappingAgentIds; +} + type AgentsDeleteOptions = { id: string; force?: boolean; @@ -79,6 +132,7 @@ export async function agentsDeleteCommand( await replaceConfigFile({ nextConfig: result.config, ...(baseHash !== undefined ? { baseHash } : {}), + writeOptions: opts.json ? { skipOutputLogs: true } : undefined, }); if (!opts.json) { logConfigUpdated(runtime); @@ -88,7 +142,16 @@ export async function agentsDeleteCommand( await purgeAgentSessionStoreEntries(cfg, agentId); const quietRuntime = opts.json ? createQuietRuntime(runtime) : runtime; - await moveToTrash(workspaceDir, quietRuntime); + // Only trash the workspace if no other agent can depend on that path (#70890). + const workspaceSharedWith = findOverlappingWorkspaceAgentIds(cfg, agentId, workspaceDir); + const workspaceRetained = workspaceSharedWith.length > 0; + if (workspaceRetained) { + quietRuntime.log( + `Skipped workspace removal (shared with other agents: ${workspaceSharedWith.join(", ")}): ${workspaceDir}`, + ); + } else { + await moveToTrash(workspaceDir, quietRuntime); + } await moveToTrash(agentDir, quietRuntime); await moveToTrash(sessionsDir, quietRuntime); @@ -96,6 +159,9 @@ export async function agentsDeleteCommand( writeRuntimeJson(runtime, { agentId, workspace: workspaceDir, + workspaceRetained: workspaceRetained || undefined, + workspaceRetainedReason: workspaceRetained ? "shared" : undefined, + workspaceSharedWith: workspaceRetained ? workspaceSharedWith : undefined, agentDir, sessionsDir, removedBindings: result.removedBindings, diff --git a/src/commands/agents.delete.test.ts b/src/commands/agents.delete.test.ts index 1f0537dcb4c..69543f0dbeb 100644 --- a/src/commands/agents.delete.test.ts +++ b/src/commands/agents.delete.test.ts @@ -61,6 +61,15 @@ function expectSessionStore( expect(loadSessionStore(storePath, { skipCache: true })).toEqual(sessions); } +function readJsonLogs(): Array> { + return runtime.log.mock.calls + .filter((call): call is [string, ...unknown[]] => { + const arg = call[0]; + return typeof arg === "string" && arg.startsWith("{"); + }) + .map((call) => JSON.parse(call[0]) as Record); +} + describe("agents delete command", () => { beforeEach(() => { configMocks.readConfigFileSnapshot.mockReset(); @@ -175,4 +184,202 @@ describe("agents delete command", () => { }); }); }); + + it("skips workspace removal when another agent shares the same workspace (#70890)", async () => { + await withStateDirEnv("openclaw-agents-delete-shared-workspace-", async ({ stateDir }) => { + const sharedWorkspace = path.join(stateDir, "workspace-shared"); + await fs.mkdir(sharedWorkspace, { recursive: true }); + + const now = Date.now(); + const cfg: OpenClawConfig = { + agents: { + list: [ + { id: "main", workspace: sharedWorkspace }, + { id: "ops", workspace: sharedWorkspace }, + ], + }, + } satisfies OpenClawConfig; + await arrangeAgentsDeleteTest({ + stateDir, + cfg, + deletedAgentId: "ops", + sessions: { + "agent:ops:main": { sessionId: "sess-ops-main", updatedAt: now + 1 }, + "agent:main:main": { sessionId: "sess-main", updatedAt: now + 2 }, + }, + }); + + await agentsDeleteCommand({ id: "ops", force: true, json: true }, runtime); + + // Workspace should still exist — it was shared + const stat = await fs.stat(sharedWorkspace).catch(() => null); + expect(stat).not.toBeNull(); + + // The JSON output should report why the workspace was retained. + const jsonOutput = readJsonLogs(); + expect(jsonOutput).toHaveLength(1); + expect(jsonOutput[0]).toMatchObject({ + workspaceRetained: true, + workspaceRetainedReason: "shared", + workspaceSharedWith: ["main"], + }); + expect(processMocks.runCommandWithTimeout).not.toHaveBeenCalledWith( + ["trash", sharedWorkspace], + { timeoutMs: 5000 }, + ); + }); + }); + + it("skips workspace removal when another agent workspace overlaps a child path (#70890)", async () => { + await withStateDirEnv("openclaw-agents-delete-overlapping-workspace-", async ({ stateDir }) => { + const sharedWorkspace = path.join(stateDir, "workspace-shared"); + const childWorkspace = path.join(sharedWorkspace, "ops-child"); + await fs.mkdir(childWorkspace, { recursive: true }); + + const now = Date.now(); + const cfg: OpenClawConfig = { + agents: { + list: [ + { id: "main", workspace: sharedWorkspace }, + { id: "ops", workspace: childWorkspace }, + ], + }, + } satisfies OpenClawConfig; + await arrangeAgentsDeleteTest({ + stateDir, + cfg, + deletedAgentId: "ops", + sessions: { + "agent:ops:main": { sessionId: "sess-ops-main", updatedAt: now + 1 }, + "agent:main:main": { sessionId: "sess-main", updatedAt: now + 2 }, + }, + }); + + await agentsDeleteCommand({ id: "ops", force: true, json: true }, runtime); + + expect(readJsonLogs()[0]).toMatchObject({ + workspaceRetained: true, + workspaceSharedWith: ["main"], + }); + expect(processMocks.runCommandWithTimeout).not.toHaveBeenCalledWith( + ["trash", childWorkspace], + { timeoutMs: 5000 }, + ); + }); + }); + + it("skips workspace removal when deleting a parent workspace that contains another agent workspace (#70890)", async () => { + await withStateDirEnv("openclaw-agents-delete-parent-workspace-", async ({ stateDir }) => { + const sharedWorkspace = path.join(stateDir, "workspace-shared"); + const childWorkspace = path.join(sharedWorkspace, "main-child"); + await fs.mkdir(childWorkspace, { recursive: true }); + + const now = Date.now(); + const cfg: OpenClawConfig = { + agents: { + list: [ + { id: "main", workspace: childWorkspace }, + { id: "ops", workspace: sharedWorkspace }, + ], + }, + } satisfies OpenClawConfig; + await arrangeAgentsDeleteTest({ + stateDir, + cfg, + deletedAgentId: "ops", + sessions: { + "agent:ops:main": { sessionId: "sess-ops-main", updatedAt: now + 1 }, + "agent:main:main": { sessionId: "sess-main", updatedAt: now + 2 }, + }, + }); + + await agentsDeleteCommand({ id: "ops", force: true, json: true }, runtime); + + expect(readJsonLogs()[0]).toMatchObject({ + workspaceRetained: true, + workspaceSharedWith: ["main"], + }); + expect(processMocks.runCommandWithTimeout).not.toHaveBeenCalledWith( + ["trash", sharedWorkspace], + { timeoutMs: 5000 }, + ); + }); + }); + + it.runIf(process.platform !== "win32")( + "skips workspace removal when another agent reaches the same directory through a symlink (#70890)", + async () => { + await withStateDirEnv("openclaw-agents-delete-symlink-workspace-", async ({ stateDir }) => { + const realWorkspace = path.join(stateDir, "workspace-real"); + const aliasWorkspace = path.join(stateDir, "workspace-alias"); + await fs.mkdir(realWorkspace, { recursive: true }); + await fs.symlink(realWorkspace, aliasWorkspace, "dir"); + + const now = Date.now(); + const cfg: OpenClawConfig = { + agents: { + list: [ + { id: "main", workspace: realWorkspace }, + { id: "ops", workspace: aliasWorkspace }, + ], + }, + } satisfies OpenClawConfig; + await arrangeAgentsDeleteTest({ + stateDir, + cfg, + deletedAgentId: "ops", + sessions: { + "agent:ops:main": { sessionId: "sess-ops-main", updatedAt: now + 1 }, + "agent:main:main": { sessionId: "sess-main", updatedAt: now + 2 }, + }, + }); + + await agentsDeleteCommand({ id: "ops", force: true, json: true }, runtime); + + expect(readJsonLogs()[0]).toMatchObject({ + workspaceRetained: true, + workspaceSharedWith: ["main"], + }); + expect(processMocks.runCommandWithTimeout).not.toHaveBeenCalledWith( + ["trash", aliasWorkspace], + { timeoutMs: 5000 }, + ); + }); + }, + ); + + it("trashes workspace when no other agent shares it", async () => { + await withStateDirEnv("openclaw-agents-delete-unique-workspace-", async ({ stateDir }) => { + const opsWorkspace = path.join(stateDir, "workspace-ops"); + const mainWorkspace = path.join(stateDir, "workspace-main"); + await fs.mkdir(opsWorkspace, { recursive: true }); + await fs.mkdir(mainWorkspace, { recursive: true }); + + const now = Date.now(); + const cfg: OpenClawConfig = { + agents: { + list: [ + { id: "main", workspace: mainWorkspace }, + { id: "ops", workspace: opsWorkspace }, + ], + }, + } satisfies OpenClawConfig; + await arrangeAgentsDeleteTest({ + stateDir, + cfg, + deletedAgentId: "ops", + sessions: { + "agent:ops:main": { sessionId: "sess-ops-main", updatedAt: now + 1 }, + "agent:main:main": { sessionId: "sess-main", updatedAt: now + 2 }, + }, + }); + + await agentsDeleteCommand({ id: "ops", force: true, json: true }, runtime); + + // trash command should have been called for the workspace + expect(processMocks.runCommandWithTimeout).toHaveBeenCalledWith(["trash", opsWorkspace], { + timeoutMs: 5000, + }); + }); + }); }); diff --git a/src/config/io.ts b/src/config/io.ts index 5d17a4a2bab..45f4e8f6318 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -168,6 +168,11 @@ export type ConfigWriteOptions = { * Normal writers must keep this false so clobbers are rejected before disk commit. */ allowDestructiveWrite?: boolean; + /** + * Suppress human-readable output logs (overwrite/anomaly messages). + * Useful when the caller wants machine-readable output only (--json mode). + */ + skipOutputLogs?: boolean; }; export type ReadConfigFileSnapshotForWriteResult = { @@ -1741,6 +1746,9 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { if (!snapshot.exists) { return; } + if (options.skipOutputLogs) { + return; + } const isVitest = deps.env.VITEST === "true"; const shouldLogInVitest = deps.env.OPENCLAW_TEST_CONFIG_OVERWRITE_LOG === "1"; if (isVitest && !shouldLogInVitest) { @@ -1759,6 +1767,9 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { if (suspiciousReasons.length === 0) { return; } + if (options.skipOutputLogs) { + return; + } // Tests often write minimal configs (missing meta, etc); keep output quiet unless requested. const isVitest = deps.env.VITEST === "true"; const shouldLogInVitest = deps.env.OPENCLAW_TEST_CONFIG_WRITE_ANOMALY_LOG === "1"; @@ -2042,6 +2053,7 @@ export async function writeConfigFile( unsetPaths: options.unsetPaths, allowDestructiveWrite: options.allowDestructiveWrite, skipRuntimeSnapshotRefresh: options.skipRuntimeSnapshotRefresh, + skipOutputLogs: options.skipOutputLogs, }); if ( options.skipRuntimeSnapshotRefresh && diff --git a/src/config/io.write-config.test.ts b/src/config/io.write-config.test.ts index 3e3c03f7040..56735106617 100644 --- a/src/config/io.write-config.test.ts +++ b/src/config/io.write-config.test.ts @@ -222,6 +222,42 @@ describe("config io write", () => { }); }); + it("suppresses overwrite audit output when skipOutputLogs is set", async () => { + await withSuiteHome(async (home) => { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile( + configPath, + `${JSON.stringify({ gateway: { mode: "local", port: 18789 } }, null, 2)}\n`, + "utf-8", + ); + const warn = vi.fn(); + const io = createConfigIO({ + env: { + VITEST: "true", + OPENCLAW_TEST_CONFIG_OVERWRITE_LOG: "1", + } as NodeJS.ProcessEnv, + homedir: () => home, + logger: { + warn, + error: vi.fn(), + }, + }); + + await io.writeConfigFile( + { + gateway: { mode: "local", port: 18790 }, + }, + { skipOutputLogs: true }, + ); + + const overwriteLogs = warn.mock.calls.filter( + (call) => typeof call[0] === "string" && call[0].startsWith("Config overwrite:"), + ); + expect(overwriteLogs).toHaveLength(0); + }); + }); + it("preserves root $schema during partial writes", async () => { await withSuiteHome(async (home) => { const configPath = path.join(home, ".openclaw", "openclaw.json");