From a99e520bec25c4bfd8eee3167cd2601b5020ed0d Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 5 May 2026 18:26:07 -0700 Subject: [PATCH] fix(exec): clean approval pending warnings --- CHANGELOG.md | 1 + docs/gateway/config-tools.md | 6 + docs/tools/exec.md | 2 + ...ash-tools.exec-foreground-failures.test.ts | 18 +++ .../bash-tools.exec.approval-id.test.ts | 31 +++++ src/agents/bash-tools.exec.ts | 11 +- src/commands/doctor/repair-sequencing.ts | 2 + .../doctor/shared/preview-warnings.ts | 2 + .../tool-companion-allowlist-repair.test.ts | 113 +++++++++++++++++ .../shared/tool-companion-allowlist-repair.ts | 119 ++++++++++++++++++ 10 files changed, 302 insertions(+), 3 deletions(-) create mode 100644 src/commands/doctor/shared/tool-companion-allowlist-repair.test.ts create mode 100644 src/commands/doctor/shared/tool-companion-allowlist-repair.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index aed86c081a2..ebcc83509ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Slack: preserve Socket Mode SDK error context and structured Slack API fields in reconnect logs, so startup failures no longer collapse to a bare `unknown error`. +- Exec/Doctor: stop showing the misleading `background execution is disabled` warning on approval-pending chat exec prompts and repair restricted-profile configs that allowed `exec`/`write` without the companion `process`/`edit` tools. Thanks @vincentkoc. - Plugins/diagnostics: make source-only TypeScript package warnings actionable by explaining that missing compiled runtime output is a publisher packaging issue and pointing users to update/reinstall or disable/uninstall the plugin. Fixes #77835. Thanks @googlerest. - TUI: skip the generic CLI respawn wrapper for interactive launches, exit cleanly on terminal loss, and refuse to restore heartbeat sessions as the remembered chat session, preventing stale heartbeat history and orphaned `openclaw-tui` processes on first boot. Thanks @vincentkoc. - Doctor/sessions: move heartbeat-poisoned default main session store entries to recovery keys and clear stale TUI restore pointers, so `doctor --fix` can repair instances already stuck on `agent:main:main` heartbeat history. Thanks @vincentkoc. diff --git a/docs/gateway/config-tools.md b/docs/gateway/config-tools.md index 33df72a8b46..b93e202adfa 100644 --- a/docs/gateway/config-tools.md +++ b/docs/gateway/config-tools.md @@ -27,6 +27,12 @@ Local onboarding defaults new local configs to `tools.profile: "coding"` when un | `messaging` | `group:messaging`, `sessions_list`, `sessions_history`, `sessions_send`, `session_status` | | `full` | No restriction (same as unset) | +When you add runtime or filesystem tools to a restricted profile, prefer the matching group +(`group:runtime` or `group:fs`) instead of only one tool from that group. `exec` relies on +`process` for background status/follow-up handling, and `write` commonly pairs with `edit` for +file updates. `openclaw doctor --fix` repairs older partial `alsoAllow` configs that already opted +into `exec`/`write` but missed those companion tools. + ### Tool groups | Group | Tools | diff --git a/docs/tools/exec.md b/docs/tools/exec.md index 24a9c9a452a..4fb7e3acc55 100644 --- a/docs/tools/exec.md +++ b/docs/tools/exec.md @@ -9,6 +9,8 @@ title: "Exec tool" Run shell commands in the workspace. Supports foreground + background execution via `process`. If `process` is disallowed, `exec` runs synchronously and ignores `yieldMs`/`background`. Background sessions are scoped per agent; `process` only sees sessions from the same agent. +Approval-gated gateway/node execs are an exception: the approval prompt returns immediately, and +the approved command continues through the exec approval follow-up path. ## Parameters diff --git a/src/agents/bash-tools.exec-foreground-failures.test.ts b/src/agents/bash-tools.exec-foreground-failures.test.ts index 4ca357b67de..402c27962aa 100644 --- a/src/agents/bash-tools.exec-foreground-failures.test.ts +++ b/src/agents/bash-tools.exec-foreground-failures.test.ts @@ -51,6 +51,24 @@ describe("exec foreground failures", () => { expect((result.details as { durationMs?: number }).durationMs).toEqual(expect.any(Number)); }); + it("keeps the background-disabled warning when exec actually runs synchronously", async () => { + const tool = createExecTool({ + security: "full", + ask: "off", + allowBackground: false, + }); + + const result = await tool.execute("call-background-disabled-foreground", { + command: isWin ? "Write-Output ok" : "printf ok", + background: true, + }); + + expect(result.details.status).toBe("completed"); + expect((result.content[0] as { text?: string }).text).toContain( + "Warning: background execution is disabled; running synchronously.", + ); + }); + it("rejects invalid host values before launching a command", async () => { const tool = createExecTool({ security: "full", diff --git a/src/agents/bash-tools.exec.approval-id.test.ts b/src/agents/bash-tools.exec.approval-id.test.ts index 52fa7dc525c..71493eafc40 100644 --- a/src/agents/bash-tools.exec.approval-id.test.ts +++ b/src/agents/bash-tools.exec.approval-id.test.ts @@ -949,6 +949,37 @@ describe("exec approvals", () => { ); }); + it("does not show the background-disabled warning while gateway exec is waiting for approval", async () => { + let approvalRequest: Record | undefined; + vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => { + if (method === "exec.approval.request") { + approvalRequest = params as Record; + return acceptedApprovalResponse(params); + } + if (method === "exec.approval.waitDecision") { + return { decision: "deny" }; + } + return { ok: true }; + }); + + const tool = createExecTool({ + host: "gateway", + ask: "always", + security: "full", + allowBackground: false, + approvalRunningNoticeMs: 0, + }); + + const result = await tool.execute("call-gw-background-approval", { + command: "echo ok", + background: true, + }); + + expect(result.details.status).toBe("approval-pending"); + expect(getResultText(result)).not.toContain("background execution is disabled"); + expect(approvalRequest?.warningText).toBeUndefined(); + }); + it("continues the original agent session after approved gateway exec completes with an external route", async () => { const agentCalls: Array> = []; diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index 7b7eb058ae1..6e214be2714 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -1266,9 +1266,10 @@ export function createExecTool( let execCommandOverride: string | undefined; const backgroundRequested = params.background === true; const yieldRequested = typeof params.yieldMs === "number"; - if (!allowBackground && (backgroundRequested || yieldRequested)) { - warnings.push("Warning: background execution is disabled; running synchronously."); - } + const backgroundDisabledWarning = + !allowBackground && (backgroundRequested || yieldRequested) + ? "Warning: background execution is disabled; running synchronously." + : undefined; const yieldWindow = allowBackground ? backgroundRequested ? 0 @@ -1548,6 +1549,10 @@ export function createExecTool( } } + if (backgroundDisabledWarning) { + warnings.push(backgroundDisabledWarning); + } + const explicitTimeoutSec = typeof params.timeout === "number" ? params.timeout : null; const effectiveTimeout = explicitTimeoutSec ?? defaultTimeoutSec; const getWarningText = () => (warnings.length ? `${warnings.join("\n")}\n\n` : ""); diff --git a/src/commands/doctor/repair-sequencing.ts b/src/commands/doctor/repair-sequencing.ts index 8ad475a8ebd..190fb3892f6 100644 --- a/src/commands/doctor/repair-sequencing.ts +++ b/src/commands/doctor/repair-sequencing.ts @@ -20,6 +20,7 @@ import { repairMissingConfiguredPluginInstalls } from "./shared/missing-configur import { maybeRepairOpenPolicyAllowFrom } from "./shared/open-policy-allowfrom.js"; import { cleanupLegacyPluginDependencyState } from "./shared/plugin-dependency-cleanup.js"; import { maybeRepairStalePluginConfig } from "./shared/stale-plugin-config.js"; +import { maybeRepairToolCompanionAllowlists } from "./shared/tool-companion-allowlist-repair.js"; const UPDATE_IN_PROGRESS_ENV = "OPENCLAW_UPDATE_IN_PROGRESS"; @@ -112,6 +113,7 @@ export async function runDoctorRepairSequence(params: { } applyMutation(maybeRepairLegacyToolsBySenderKeys(state.candidate)); + applyMutation(maybeRepairToolCompanionAllowlists(state.candidate)); applyMutation(maybeRepairExecSafeBinProfiles(state.candidate)); const pluginDependencyCleanup = await cleanupLegacyPluginDependencyState({ env }); if (pluginDependencyCleanup.changes.length > 0) { diff --git a/src/commands/doctor/shared/preview-warnings.ts b/src/commands/doctor/shared/preview-warnings.ts index ddd4c2accc4..0e0bcc3c553 100644 --- a/src/commands/doctor/shared/preview-warnings.ts +++ b/src/commands/doctor/shared/preview-warnings.ts @@ -4,6 +4,7 @@ import { mergeAlsoAllowPolicy, resolveToolProfilePolicy } from "../../../agents/ import type { OpenClawConfig } from "../../../config/types.openclaw.js"; import type { AgentToolsConfig, ToolsConfig } from "../../../config/types.tools.js"; import { createLazyImportLoader } from "../../../shared/lazy-promise.js"; +import { collectToolCompanionAllowlistWarnings } from "./tool-companion-allowlist-repair.js"; type ChannelDoctorModule = typeof import("./channel-doctor.js"); @@ -194,6 +195,7 @@ export async function collectDoctorPreviewWarnings(params: { const hasChannelConfig = hasChannels(params.cfg); const hasPluginConfig = hasPlugins(params.cfg); + warnings.push(...collectToolCompanionAllowlistWarnings(params.cfg, params.doctorFixCommand)); warnings.push(...collectVisibleReplyToolPolicyWarnings(params.cfg)); const channelPluginRuntime = diff --git a/src/commands/doctor/shared/tool-companion-allowlist-repair.test.ts b/src/commands/doctor/shared/tool-companion-allowlist-repair.test.ts new file mode 100644 index 00000000000..f2cf8fcdf64 --- /dev/null +++ b/src/commands/doctor/shared/tool-companion-allowlist-repair.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../../../config/types.openclaw.js"; +import { + collectToolCompanionAllowlistWarnings, + maybeRepairToolCompanionAllowlists, +} from "./tool-companion-allowlist-repair.js"; + +describe("tool companion allowlist repair", () => { + it("warns when restricted profiles allow exec/write without companion tools", () => { + const cfg: OpenClawConfig = { + tools: { + profile: "messaging", + alsoAllow: ["exec", "read", "write", "session_status"], + exec: { security: "allowlist", ask: "on-miss" }, + fs: { workspaceOnly: true }, + }, + }; + + const warnings = collectToolCompanionAllowlistWarnings(cfg, "openclaw doctor --fix"); + + expect(warnings.join("\n")).toContain("tools.alsoAllow"); + expect(warnings.join("\n")).toContain('"process"'); + expect(warnings.join("\n")).toContain('"edit"'); + expect(warnings.join("\n")).toContain("openclaw doctor --fix"); + }); + + it("repairs explicit global companion omissions without changing unrelated tools", () => { + const cfg: OpenClawConfig = { + tools: { + profile: "messaging", + alsoAllow: ["exec", "read", "write", "web_fetch"], + exec: { security: "allowlist", ask: "on-miss" }, + fs: { workspaceOnly: true }, + }, + }; + + const result = maybeRepairToolCompanionAllowlists(cfg); + + expect(result.config.tools?.alsoAllow).toEqual([ + "exec", + "read", + "write", + "web_fetch", + "process", + "edit", + ]); + expect(result.changes).toEqual(['Added "process", "edit" to tools.alsoAllow.']); + }); + + it("uses inherited global exec/fs sections for agent-level allowlists", () => { + const cfg: OpenClawConfig = { + tools: { + exec: { security: "allowlist" }, + fs: { workspaceOnly: true }, + }, + agents: { + list: [ + { + id: "zollie", + tools: { + profile: "messaging", + alsoAllow: ["exec", "write"], + }, + }, + ], + }, + }; + + const result = maybeRepairToolCompanionAllowlists(cfg); + + expect(result.config.agents?.list?.[0]?.tools?.alsoAllow).toEqual([ + "exec", + "write", + "process", + "edit", + ]); + }); + + it("does not rewrite complete groups or configs without explicit partial opt-ins", () => { + const grouped: OpenClawConfig = { + tools: { + profile: "messaging", + alsoAllow: ["group:runtime", "group:fs"], + exec: {}, + fs: {}, + }, + }; + const implicitOnly: OpenClawConfig = { + tools: { + profile: "messaging", + exec: {}, + fs: {}, + }, + }; + + expect(maybeRepairToolCompanionAllowlists(grouped).changes).toEqual([]); + expect(maybeRepairToolCompanionAllowlists(implicitOnly).changes).toEqual([]); + }); + + it("does not warn for coding profiles that already include companion tools", () => { + const cfg: OpenClawConfig = { + tools: { + profile: "coding", + alsoAllow: ["exec", "write"], + exec: {}, + fs: {}, + }, + }; + + expect(collectToolCompanionAllowlistWarnings(cfg, "openclaw doctor --fix")).toEqual([]); + expect(maybeRepairToolCompanionAllowlists(cfg).changes).toEqual([]); + }); +}); diff --git a/src/commands/doctor/shared/tool-companion-allowlist-repair.ts b/src/commands/doctor/shared/tool-companion-allowlist-repair.ts new file mode 100644 index 00000000000..9bede1eee26 --- /dev/null +++ b/src/commands/doctor/shared/tool-companion-allowlist-repair.ts @@ -0,0 +1,119 @@ +import { expandToolGroups, normalizeToolList } from "../../../agents/tool-policy.js"; +import type { OpenClawConfig } from "../../../config/types.openclaw.js"; +import type { AgentToolsConfig, ToolsConfig } from "../../../config/types.tools.js"; + +type ToolConfigLike = ToolsConfig | AgentToolsConfig; + +type ToolCompanionFinding = { + path: string; + additions: string[]; +}; + +function hasExplicitToolSection(value: unknown): boolean { + return value !== undefined && value !== null; +} + +function companionAdditionsForTools(params: { + tools: ToolConfigLike | undefined; + profile?: string; + inheritedExec?: unknown; + inheritedFs?: unknown; +}): string[] { + if (params.profile !== "messaging" && params.profile !== "minimal") { + return []; + } + const alsoAllow = Array.isArray(params.tools?.alsoAllow) ? params.tools.alsoAllow : undefined; + if (!alsoAllow || alsoAllow.length === 0) { + return []; + } + + const normalized = normalizeToolList(alsoAllow); + const expanded = new Set(expandToolGroups(alsoAllow)); + const additions: string[] = []; + const hasExecConfig = + hasExplicitToolSection(params.tools?.exec) || hasExplicitToolSection(params.inheritedExec); + const hasFsConfig = + hasExplicitToolSection(params.tools?.fs) || hasExplicitToolSection(params.inheritedFs); + + if (hasExecConfig && normalized.includes("exec") && !expanded.has("process")) { + additions.push("process"); + } + if (hasFsConfig && normalized.includes("write") && !expanded.has("edit")) { + additions.push("edit"); + } + return additions; +} + +function collectToolCompanionFindings(cfg: OpenClawConfig): ToolCompanionFinding[] { + const findings: ToolCompanionFinding[] = []; + const globalAdditions = companionAdditionsForTools({ + tools: cfg.tools, + profile: cfg.tools?.profile, + }); + if (globalAdditions.length > 0) { + findings.push({ path: "tools.alsoAllow", additions: globalAdditions }); + } + + for (const [index, agent] of (cfg.agents?.list ?? []).entries()) { + const additions = companionAdditionsForTools({ + tools: agent.tools, + profile: agent.tools?.profile ?? cfg.tools?.profile, + inheritedExec: cfg.tools?.exec, + inheritedFs: cfg.tools?.fs, + }); + if (additions.length > 0) { + findings.push({ path: `agents.list[${index}].tools.alsoAllow`, additions }); + } + } + + return findings; +} + +function formatAdditions(additions: string[]): string { + return additions.map((value) => `"${value}"`).join(", "); +} + +export function collectToolCompanionAllowlistWarnings( + cfg: OpenClawConfig, + doctorFixCommand: string, +): string[] { + return collectToolCompanionFindings(cfg).map( + (finding) => + `- ${finding.path}: add ${formatAdditions( + finding.additions, + )} so restricted profiles that already allow exec/write also expose the companion runtime/edit tools. Run "${doctorFixCommand}" to repair.`, + ); +} + +export function maybeRepairToolCompanionAllowlists(cfg: OpenClawConfig): { + config: OpenClawConfig; + changes: string[]; +} { + const findings = collectToolCompanionFindings(cfg); + if (findings.length === 0) { + return { config: cfg, changes: [] }; + } + + const next = structuredClone(cfg); + const changes: string[] = []; + for (const finding of findings) { + const target = + finding.path === "tools.alsoAllow" + ? next.tools + : next.agents?.list?.[Number(finding.path.match(/^agents\.list\[(\d+)\]/)?.[1])]?.tools; + if (!target) { + continue; + } + const current = Array.isArray(target.alsoAllow) ? target.alsoAllow : []; + const merged = [...current]; + for (const addition of finding.additions) { + if (!normalizeToolList(merged).includes(addition)) { + merged.push(addition); + } + } + target.alsoAllow = merged; + changes.push(`Added ${formatAdditions(finding.additions)} to ${finding.path}.`); + } + + return { config: next, changes }; +}