Skip to content

Commit a99e520

Browse files
committed
fix(exec): clean approval pending warnings
1 parent 7f71e84 commit a99e520

10 files changed

Lines changed: 302 additions & 3 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ Docs: https://docs.openclaw.ai
101101
### Fixes
102102

103103
- 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`.
104+
- 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.
104105
- 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.
105106
- 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.
106107
- 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.

docs/gateway/config-tools.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ Local onboarding defaults new local configs to `tools.profile: "coding"` when un
2727
| `messaging` | `group:messaging`, `sessions_list`, `sessions_history`, `sessions_send`, `session_status` |
2828
| `full` | No restriction (same as unset) |
2929

30+
When you add runtime or filesystem tools to a restricted profile, prefer the matching group
31+
(`group:runtime` or `group:fs`) instead of only one tool from that group. `exec` relies on
32+
`process` for background status/follow-up handling, and `write` commonly pairs with `edit` for
33+
file updates. `openclaw doctor --fix` repairs older partial `alsoAllow` configs that already opted
34+
into `exec`/`write` but missed those companion tools.
35+
3036
### Tool groups
3137

3238
| Group | Tools |

docs/tools/exec.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ title: "Exec tool"
99
Run shell commands in the workspace. Supports foreground + background execution via `process`.
1010
If `process` is disallowed, `exec` runs synchronously and ignores `yieldMs`/`background`.
1111
Background sessions are scoped per agent; `process` only sees sessions from the same agent.
12+
Approval-gated gateway/node execs are an exception: the approval prompt returns immediately, and
13+
the approved command continues through the exec approval follow-up path.
1214

1315
## Parameters
1416

src/agents/bash-tools.exec-foreground-failures.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,24 @@ describe("exec foreground failures", () => {
5151
expect((result.details as { durationMs?: number }).durationMs).toEqual(expect.any(Number));
5252
});
5353

54+
it("keeps the background-disabled warning when exec actually runs synchronously", async () => {
55+
const tool = createExecTool({
56+
security: "full",
57+
ask: "off",
58+
allowBackground: false,
59+
});
60+
61+
const result = await tool.execute("call-background-disabled-foreground", {
62+
command: isWin ? "Write-Output ok" : "printf ok",
63+
background: true,
64+
});
65+
66+
expect(result.details.status).toBe("completed");
67+
expect((result.content[0] as { text?: string }).text).toContain(
68+
"Warning: background execution is disabled; running synchronously.",
69+
);
70+
});
71+
5472
it("rejects invalid host values before launching a command", async () => {
5573
const tool = createExecTool({
5674
security: "full",

src/agents/bash-tools.exec.approval-id.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -949,6 +949,37 @@ describe("exec approvals", () => {
949949
);
950950
});
951951

952+
it("does not show the background-disabled warning while gateway exec is waiting for approval", async () => {
953+
let approvalRequest: Record<string, unknown> | undefined;
954+
vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => {
955+
if (method === "exec.approval.request") {
956+
approvalRequest = params as Record<string, unknown>;
957+
return acceptedApprovalResponse(params);
958+
}
959+
if (method === "exec.approval.waitDecision") {
960+
return { decision: "deny" };
961+
}
962+
return { ok: true };
963+
});
964+
965+
const tool = createExecTool({
966+
host: "gateway",
967+
ask: "always",
968+
security: "full",
969+
allowBackground: false,
970+
approvalRunningNoticeMs: 0,
971+
});
972+
973+
const result = await tool.execute("call-gw-background-approval", {
974+
command: "echo ok",
975+
background: true,
976+
});
977+
978+
expect(result.details.status).toBe("approval-pending");
979+
expect(getResultText(result)).not.toContain("background execution is disabled");
980+
expect(approvalRequest?.warningText).toBeUndefined();
981+
});
982+
952983
it("continues the original agent session after approved gateway exec completes with an external route", async () => {
953984
const agentCalls: Array<Record<string, unknown>> = [];
954985

src/agents/bash-tools.exec.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,9 +1266,10 @@ export function createExecTool(
12661266
let execCommandOverride: string | undefined;
12671267
const backgroundRequested = params.background === true;
12681268
const yieldRequested = typeof params.yieldMs === "number";
1269-
if (!allowBackground && (backgroundRequested || yieldRequested)) {
1270-
warnings.push("Warning: background execution is disabled; running synchronously.");
1271-
}
1269+
const backgroundDisabledWarning =
1270+
!allowBackground && (backgroundRequested || yieldRequested)
1271+
? "Warning: background execution is disabled; running synchronously."
1272+
: undefined;
12721273
const yieldWindow = allowBackground
12731274
? backgroundRequested
12741275
? 0
@@ -1548,6 +1549,10 @@ export function createExecTool(
15481549
}
15491550
}
15501551

1552+
if (backgroundDisabledWarning) {
1553+
warnings.push(backgroundDisabledWarning);
1554+
}
1555+
15511556
const explicitTimeoutSec = typeof params.timeout === "number" ? params.timeout : null;
15521557
const effectiveTimeout = explicitTimeoutSec ?? defaultTimeoutSec;
15531558
const getWarningText = () => (warnings.length ? `${warnings.join("\n")}\n\n` : "");

src/commands/doctor/repair-sequencing.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { repairMissingConfiguredPluginInstalls } from "./shared/missing-configur
2020
import { maybeRepairOpenPolicyAllowFrom } from "./shared/open-policy-allowfrom.js";
2121
import { cleanupLegacyPluginDependencyState } from "./shared/plugin-dependency-cleanup.js";
2222
import { maybeRepairStalePluginConfig } from "./shared/stale-plugin-config.js";
23+
import { maybeRepairToolCompanionAllowlists } from "./shared/tool-companion-allowlist-repair.js";
2324

2425
const UPDATE_IN_PROGRESS_ENV = "OPENCLAW_UPDATE_IN_PROGRESS";
2526

@@ -112,6 +113,7 @@ export async function runDoctorRepairSequence(params: {
112113
}
113114

114115
applyMutation(maybeRepairLegacyToolsBySenderKeys(state.candidate));
116+
applyMutation(maybeRepairToolCompanionAllowlists(state.candidate));
115117
applyMutation(maybeRepairExecSafeBinProfiles(state.candidate));
116118
const pluginDependencyCleanup = await cleanupLegacyPluginDependencyState({ env });
117119
if (pluginDependencyCleanup.changes.length > 0) {

src/commands/doctor/shared/preview-warnings.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { mergeAlsoAllowPolicy, resolveToolProfilePolicy } from "../../../agents/
44
import type { OpenClawConfig } from "../../../config/types.openclaw.js";
55
import type { AgentToolsConfig, ToolsConfig } from "../../../config/types.tools.js";
66
import { createLazyImportLoader } from "../../../shared/lazy-promise.js";
7+
import { collectToolCompanionAllowlistWarnings } from "./tool-companion-allowlist-repair.js";
78

89
type ChannelDoctorModule = typeof import("./channel-doctor.js");
910

@@ -194,6 +195,7 @@ export async function collectDoctorPreviewWarnings(params: {
194195
const hasChannelConfig = hasChannels(params.cfg);
195196
const hasPluginConfig = hasPlugins(params.cfg);
196197

198+
warnings.push(...collectToolCompanionAllowlistWarnings(params.cfg, params.doctorFixCommand));
197199
warnings.push(...collectVisibleReplyToolPolicyWarnings(params.cfg));
198200

199201
const channelPluginRuntime =
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import { describe, expect, it } from "vitest";
2+
import type { OpenClawConfig } from "../../../config/types.openclaw.js";
3+
import {
4+
collectToolCompanionAllowlistWarnings,
5+
maybeRepairToolCompanionAllowlists,
6+
} from "./tool-companion-allowlist-repair.js";
7+
8+
describe("tool companion allowlist repair", () => {
9+
it("warns when restricted profiles allow exec/write without companion tools", () => {
10+
const cfg: OpenClawConfig = {
11+
tools: {
12+
profile: "messaging",
13+
alsoAllow: ["exec", "read", "write", "session_status"],
14+
exec: { security: "allowlist", ask: "on-miss" },
15+
fs: { workspaceOnly: true },
16+
},
17+
};
18+
19+
const warnings = collectToolCompanionAllowlistWarnings(cfg, "openclaw doctor --fix");
20+
21+
expect(warnings.join("\n")).toContain("tools.alsoAllow");
22+
expect(warnings.join("\n")).toContain('"process"');
23+
expect(warnings.join("\n")).toContain('"edit"');
24+
expect(warnings.join("\n")).toContain("openclaw doctor --fix");
25+
});
26+
27+
it("repairs explicit global companion omissions without changing unrelated tools", () => {
28+
const cfg: OpenClawConfig = {
29+
tools: {
30+
profile: "messaging",
31+
alsoAllow: ["exec", "read", "write", "web_fetch"],
32+
exec: { security: "allowlist", ask: "on-miss" },
33+
fs: { workspaceOnly: true },
34+
},
35+
};
36+
37+
const result = maybeRepairToolCompanionAllowlists(cfg);
38+
39+
expect(result.config.tools?.alsoAllow).toEqual([
40+
"exec",
41+
"read",
42+
"write",
43+
"web_fetch",
44+
"process",
45+
"edit",
46+
]);
47+
expect(result.changes).toEqual(['Added "process", "edit" to tools.alsoAllow.']);
48+
});
49+
50+
it("uses inherited global exec/fs sections for agent-level allowlists", () => {
51+
const cfg: OpenClawConfig = {
52+
tools: {
53+
exec: { security: "allowlist" },
54+
fs: { workspaceOnly: true },
55+
},
56+
agents: {
57+
list: [
58+
{
59+
id: "zollie",
60+
tools: {
61+
profile: "messaging",
62+
alsoAllow: ["exec", "write"],
63+
},
64+
},
65+
],
66+
},
67+
};
68+
69+
const result = maybeRepairToolCompanionAllowlists(cfg);
70+
71+
expect(result.config.agents?.list?.[0]?.tools?.alsoAllow).toEqual([
72+
"exec",
73+
"write",
74+
"process",
75+
"edit",
76+
]);
77+
});
78+
79+
it("does not rewrite complete groups or configs without explicit partial opt-ins", () => {
80+
const grouped: OpenClawConfig = {
81+
tools: {
82+
profile: "messaging",
83+
alsoAllow: ["group:runtime", "group:fs"],
84+
exec: {},
85+
fs: {},
86+
},
87+
};
88+
const implicitOnly: OpenClawConfig = {
89+
tools: {
90+
profile: "messaging",
91+
exec: {},
92+
fs: {},
93+
},
94+
};
95+
96+
expect(maybeRepairToolCompanionAllowlists(grouped).changes).toEqual([]);
97+
expect(maybeRepairToolCompanionAllowlists(implicitOnly).changes).toEqual([]);
98+
});
99+
100+
it("does not warn for coding profiles that already include companion tools", () => {
101+
const cfg: OpenClawConfig = {
102+
tools: {
103+
profile: "coding",
104+
alsoAllow: ["exec", "write"],
105+
exec: {},
106+
fs: {},
107+
},
108+
};
109+
110+
expect(collectToolCompanionAllowlistWarnings(cfg, "openclaw doctor --fix")).toEqual([]);
111+
expect(maybeRepairToolCompanionAllowlists(cfg).changes).toEqual([]);
112+
});
113+
});
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import { expandToolGroups, normalizeToolList } from "../../../agents/tool-policy.js";
2+
import type { OpenClawConfig } from "../../../config/types.openclaw.js";
3+
import type { AgentToolsConfig, ToolsConfig } from "../../../config/types.tools.js";
4+
5+
type ToolConfigLike = ToolsConfig | AgentToolsConfig;
6+
7+
type ToolCompanionFinding = {
8+
path: string;
9+
additions: string[];
10+
};
11+
12+
function hasExplicitToolSection(value: unknown): boolean {
13+
return value !== undefined && value !== null;
14+
}
15+
16+
function companionAdditionsForTools(params: {
17+
tools: ToolConfigLike | undefined;
18+
profile?: string;
19+
inheritedExec?: unknown;
20+
inheritedFs?: unknown;
21+
}): string[] {
22+
if (params.profile !== "messaging" && params.profile !== "minimal") {
23+
return [];
24+
}
25+
const alsoAllow = Array.isArray(params.tools?.alsoAllow) ? params.tools.alsoAllow : undefined;
26+
if (!alsoAllow || alsoAllow.length === 0) {
27+
return [];
28+
}
29+
30+
const normalized = normalizeToolList(alsoAllow);
31+
const expanded = new Set(expandToolGroups(alsoAllow));
32+
const additions: string[] = [];
33+
const hasExecConfig =
34+
hasExplicitToolSection(params.tools?.exec) || hasExplicitToolSection(params.inheritedExec);
35+
const hasFsConfig =
36+
hasExplicitToolSection(params.tools?.fs) || hasExplicitToolSection(params.inheritedFs);
37+
38+
if (hasExecConfig && normalized.includes("exec") && !expanded.has("process")) {
39+
additions.push("process");
40+
}
41+
if (hasFsConfig && normalized.includes("write") && !expanded.has("edit")) {
42+
additions.push("edit");
43+
}
44+
return additions;
45+
}
46+
47+
function collectToolCompanionFindings(cfg: OpenClawConfig): ToolCompanionFinding[] {
48+
const findings: ToolCompanionFinding[] = [];
49+
const globalAdditions = companionAdditionsForTools({
50+
tools: cfg.tools,
51+
profile: cfg.tools?.profile,
52+
});
53+
if (globalAdditions.length > 0) {
54+
findings.push({ path: "tools.alsoAllow", additions: globalAdditions });
55+
}
56+
57+
for (const [index, agent] of (cfg.agents?.list ?? []).entries()) {
58+
const additions = companionAdditionsForTools({
59+
tools: agent.tools,
60+
profile: agent.tools?.profile ?? cfg.tools?.profile,
61+
inheritedExec: cfg.tools?.exec,
62+
inheritedFs: cfg.tools?.fs,
63+
});
64+
if (additions.length > 0) {
65+
findings.push({ path: `agents.list[${index}].tools.alsoAllow`, additions });
66+
}
67+
}
68+
69+
return findings;
70+
}
71+
72+
function formatAdditions(additions: string[]): string {
73+
return additions.map((value) => `"${value}"`).join(", ");
74+
}
75+
76+
export function collectToolCompanionAllowlistWarnings(
77+
cfg: OpenClawConfig,
78+
doctorFixCommand: string,
79+
): string[] {
80+
return collectToolCompanionFindings(cfg).map(
81+
(finding) =>
82+
`- ${finding.path}: add ${formatAdditions(
83+
finding.additions,
84+
)} so restricted profiles that already allow exec/write also expose the companion runtime/edit tools. Run "${doctorFixCommand}" to repair.`,
85+
);
86+
}
87+
88+
export function maybeRepairToolCompanionAllowlists(cfg: OpenClawConfig): {
89+
config: OpenClawConfig;
90+
changes: string[];
91+
} {
92+
const findings = collectToolCompanionFindings(cfg);
93+
if (findings.length === 0) {
94+
return { config: cfg, changes: [] };
95+
}
96+
97+
const next = structuredClone(cfg);
98+
const changes: string[] = [];
99+
for (const finding of findings) {
100+
const target =
101+
finding.path === "tools.alsoAllow"
102+
? next.tools
103+
: next.agents?.list?.[Number(finding.path.match(/^agents\.list\[(\d+)\]/)?.[1])]?.tools;
104+
if (!target) {
105+
continue;
106+
}
107+
const current = Array.isArray(target.alsoAllow) ? target.alsoAllow : [];
108+
const merged = [...current];
109+
for (const addition of finding.additions) {
110+
if (!normalizeToolList(merged).includes(addition)) {
111+
merged.push(addition);
112+
}
113+
}
114+
target.alsoAllow = merged;
115+
changes.push(`Added ${formatAdditions(finding.additions)} to ${finding.path}.`);
116+
}
117+
118+
return { config: next, changes };
119+
}

0 commit comments

Comments
 (0)