mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:40:44 +00:00
fix: retain shared agent workspaces (#70897)
Fixes #70889 and #70890. Retains overlapping/shared agent workspaces during `openclaw agents delete`, keeps `--json` output machine-readable, and repairs the stale hook-runner test harness mock that blocked CI. Thanks @kaseonedge.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -61,6 +61,15 @@ function expectSessionStore(
|
||||
expect(loadSessionStore(storePath, { skipCache: true })).toEqual(sessions);
|
||||
}
|
||||
|
||||
function readJsonLogs(): Array<Record<string, unknown>> {
|
||||
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<string, unknown>);
|
||||
}
|
||||
|
||||
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,
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 &&
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user