mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-06 05:51:15 +08:00
fix: require configured subagent allowlist targets (#85154)
* fix subagent allowlists to configured agents * add changelog for subagent allowlist fix
This commit is contained in:
@@ -53,6 +53,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Codex app-server: mark missing turn completion after observed execution as replay-unsafe and release the session so follow-up turns can run. Fixes #84076. (#85107) Thanks @joshavant.
|
||||
- Codex app-server: add a dedicated post-tool raw assistant completion idle timeout config so trusted heavy turns can wait longer after tool handoff without weakening final assistant release.
|
||||
- Matrix: keep explicitly configured two-person rooms on the room route before stale `m.direct` or strict two-member DM fallback can bypass mention gating. Fixes #85017. (#85137) Thanks @joshavant.
|
||||
- Agents/subagents: require explicit subagent allowlist targets to be configured agents so stale deleted-agent ids are omitted from `agents_list` and rejected by `sessions_spawn`. Fixes #84811. (#85154) Thanks @joshavant.
|
||||
- PDF tool: time out idle remote PDF body reads after 120 seconds so stalled remote documents return an error instead of wedging the session. Fixes #68649. (#84768) Thanks @luoyanglang.
|
||||
- Diagnostics/OpenTelemetry plugin: suppress handled OTLP exporter promise rejections so collector shutdowns no longer crash the Gateway. (#81085) Thanks @luoyanglang.
|
||||
- Agents/exec: omit raw command text and env values from denied exec failure logs while keeping safe correlation metadata. Fixes #85049. (#85140) Thanks @joshavant.
|
||||
|
||||
@@ -1093,7 +1093,7 @@ for provider examples and precedence.
|
||||
- `runtime`: optional per-agent runtime descriptor. Use `type: "acp"` with `runtime.acp` defaults (`agent`, `backend`, `mode`, `cwd`) when the agent should default to ACP harness sessions.
|
||||
- `identity.avatar`: workspace-relative path, `http(s)` URL, or `data:` URI.
|
||||
- `identity` derives defaults: `ackReaction` from `emoji`, `mentionPatterns` from `name`/`emoji`.
|
||||
- `subagents.allowAgents`: allowlist of agent ids for explicit `sessions_spawn.agentId` targets (`["*"]` = any configured target; default: same agent only). Include the requester id when self-targeted `agentId` calls should be allowed.
|
||||
- `subagents.allowAgents`: allowlist of configured agent ids for explicit `sessions_spawn.agentId` targets (`["*"]` = any configured target; default: same agent only). Include the requester id when self-targeted `agentId` calls should be allowed. Stale entries whose agent config was deleted are rejected by `sessions_spawn` and omitted from `agents_list`; run `openclaw doctor --fix` to clean them up, or add a minimal `agents.list[]` entry if that target should remain spawnable while inheriting defaults.
|
||||
- Sandbox inheritance guard: if the requester session is sandboxed, `sessions_spawn` rejects targets that would run unsandboxed.
|
||||
- `subagents.requireAgentId`: when true, block `sessions_spawn` calls that omit `agentId` (forces explicit profile selection; default: false).
|
||||
|
||||
|
||||
@@ -433,7 +433,7 @@ Experimental built-in tool flags. Default off unless a strict-agentic GPT-5 auto
|
||||
```
|
||||
|
||||
- `model`: default model for spawned sub-agents. If omitted, sub-agents inherit the caller's model.
|
||||
- `allowAgents`: default allowlist of target agent ids for `sessions_spawn` when the requester agent does not set its own `subagents.allowAgents` (`["*"]` = any configured target; default: same agent only).
|
||||
- `allowAgents`: default allowlist of configured target agent ids for `sessions_spawn` when the requester agent does not set its own `subagents.allowAgents` (`["*"]` = any configured target; default: same agent only). Stale entries whose agent config was deleted are rejected by `sessions_spawn` and omitted from `agents_list`; run `openclaw doctor --fix` to clean them up.
|
||||
- `runTimeoutSeconds`: default timeout (seconds) for `sessions_spawn` when the tool call omits `runTimeoutSeconds`. `0` means no timeout.
|
||||
- `announceTimeoutMs`: per-call timeout (milliseconds) for gateway `agent` announce delivery attempts. Default: `120000`. Transient retries can make the total announce wait longer than one configured timeout.
|
||||
- Per-subagent tool policy: `tools.subagents.tools.allow` / `tools.subagents.tools.deny`.
|
||||
|
||||
@@ -190,7 +190,7 @@ Per-agent overrides use `agents.list[].subagents.delegationMode`.
|
||||
Optional human-readable label.
|
||||
</ParamField>
|
||||
<ParamField path="agentId" type="string">
|
||||
Spawn under another agent id when allowed by `subagents.allowAgents`.
|
||||
Spawn under another configured agent id when allowed by `subagents.allowAgents`.
|
||||
</ParamField>
|
||||
<ParamField path="runtime" type='"subagent" | "acp"' default="subagent">
|
||||
`acp` is only for external ACP harnesses (`claude`, `droid`, `gemini`, `opencode`, or explicitly requested Codex ACP/acpx) and for `agents.list[]` entries whose `runtime.type` is `acp`.
|
||||
@@ -340,10 +340,10 @@ See [Configuration reference](/gateway/configuration-reference) and
|
||||
### Allowlist
|
||||
|
||||
<ParamField path="agents.list[].subagents.allowAgents" type="string[]">
|
||||
List of agent ids that can be targeted via explicit `agentId` (`["*"]` allows any configured target). Default: only the requester agent. If you set a list and still want the requester to spawn itself with `agentId`, include the requester id in the list.
|
||||
List of configured agent ids that can be targeted via explicit `agentId` (`["*"]` allows any configured target). Default: only the requester agent. If you set a list and still want the requester to spawn itself with `agentId`, include the requester id in the list.
|
||||
</ParamField>
|
||||
<ParamField path="agents.defaults.subagents.allowAgents" type="string[]">
|
||||
Default target-agent allowlist used when the requester agent does not set its own `subagents.allowAgents`.
|
||||
Default configured target-agent allowlist used when the requester agent does not set its own `subagents.allowAgents`.
|
||||
</ParamField>
|
||||
<ParamField path="agents.defaults.subagents.requireAgentId" type="boolean" default="false">
|
||||
Block `sessions_spawn` calls that omit `agentId` (forces explicit profile selection). Per-agent override: `agents.list[].subagents.requireAgentId`.
|
||||
@@ -362,6 +362,13 @@ Use `agents_list` to see which agent ids are currently allowed for
|
||||
model and embedded runtime metadata so callers can distinguish PI, Codex
|
||||
app-server, and other configured native runtimes.
|
||||
|
||||
`allowAgents` entries must point at configured agent ids in `agents.list[]`.
|
||||
`["*"]` means any configured target agent plus the requester. If an agent config
|
||||
is deleted but its id remains in `allowAgents`, `sessions_spawn` rejects that id
|
||||
and `agents_list` omits it. Run `openclaw doctor --fix` to clean stale
|
||||
allowlist entries, or add a minimal `agents.list[]` entry when the target should
|
||||
remain spawnable while inheriting defaults.
|
||||
|
||||
### Auto-archive
|
||||
|
||||
- Sub-agent sessions are automatically archived after `agents.defaults.subagents.archiveAfterMinutes` (default `60`).
|
||||
|
||||
@@ -126,7 +126,7 @@ describe("agents_list", () => {
|
||||
expect(agents?.map((agent) => agent.id)).toEqual(["main", "coder", "research"]);
|
||||
});
|
||||
|
||||
it("marks allowlisted-but-unconfigured agents", async () => {
|
||||
it("omits allowlisted-but-unconfigured agents", async () => {
|
||||
setConfigWithAgentList([
|
||||
{
|
||||
id: "main",
|
||||
@@ -139,8 +139,6 @@ describe("agents_list", () => {
|
||||
const tool = createTool();
|
||||
const result = await tool.execute("call4", {});
|
||||
const agents = readAgentList(result);
|
||||
expect(agents?.map((agent) => agent.id)).toEqual(["research"]);
|
||||
const research = agents?.find((agent) => agent.id === "research");
|
||||
expect(research?.configured).toBe(false);
|
||||
expect(agents?.map((agent) => agent.id)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -117,7 +117,7 @@ describe("subagent spawn allowlist + sandbox guards", () => {
|
||||
it("allows cross-agent spawning when configured", async () => {
|
||||
setConfig({
|
||||
agents: {
|
||||
list: [{ id: "main", subagents: { allowAgents: ["beta"] } }],
|
||||
list: [{ id: "main", subagents: { allowAgents: ["beta"] } }, { id: "beta" }],
|
||||
},
|
||||
});
|
||||
const result = await spawn({ agentId: "beta" });
|
||||
@@ -130,7 +130,7 @@ describe("subagent spawn allowlist + sandbox guards", () => {
|
||||
setConfig({
|
||||
agents: {
|
||||
defaults: { subagents: { allowAgents: ["beta"] } },
|
||||
list: [{ id: "main" }],
|
||||
list: [{ id: "main" }, { id: "beta" }],
|
||||
},
|
||||
});
|
||||
const result = await spawn({ agentId: "beta" });
|
||||
@@ -161,21 +161,24 @@ describe("subagent spawn allowlist + sandbox guards", () => {
|
||||
expect(hoisted.callGatewayMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows explicit unconfigured agent ids when allowlist also contains *", async () => {
|
||||
it("rejects explicit unconfigured agent ids when allowlist also contains *", async () => {
|
||||
setConfig({
|
||||
agents: {
|
||||
list: [{ id: "main", subagents: { allowAgents: ["*", "beta"] } }],
|
||||
},
|
||||
});
|
||||
const result = await spawn({ agentId: "beta" });
|
||||
expectStatus(result, "accepted");
|
||||
expectChildSessionKey(result, /^agent:beta:subagent:/);
|
||||
expectStatus(result, "forbidden");
|
||||
expect(result.error ?? "").toBe(
|
||||
'agentId "beta" is not in the configured agent registry (allowed: main)',
|
||||
);
|
||||
expect(hoisted.callGatewayMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("normalizes allowlisted agent ids", async () => {
|
||||
setConfig({
|
||||
agents: {
|
||||
list: [{ id: "main", subagents: { allowAgents: ["Research"] } }],
|
||||
list: [{ id: "main", subagents: { allowAgents: ["Research"] } }, { id: "research" }],
|
||||
},
|
||||
});
|
||||
const result = await spawn({ agentId: "research" });
|
||||
@@ -240,7 +243,10 @@ describe("subagent spawn allowlist + sandbox guards", () => {
|
||||
it("allows explicit agentId when requireAgentId is configured", async () => {
|
||||
setConfig({
|
||||
agents: {
|
||||
list: [{ id: "main", subagents: { allowAgents: ["worker"], requireAgentId: true } }],
|
||||
list: [
|
||||
{ id: "main", subagents: { allowAgents: ["worker"], requireAgentId: true } },
|
||||
{ id: "worker" },
|
||||
],
|
||||
},
|
||||
});
|
||||
const result = await spawn({ agentId: "worker" });
|
||||
@@ -294,13 +300,17 @@ describe("subagent spawn allowlist + sandbox guards", () => {
|
||||
expectStatus(result, "accepted");
|
||||
});
|
||||
|
||||
it("allows allowlisted-but-unconfigured agentId", async () => {
|
||||
it("rejects allowlisted-but-unconfigured agentId", async () => {
|
||||
setConfig({
|
||||
agents: {
|
||||
list: [{ id: "main", subagents: { allowAgents: ["research"] } }],
|
||||
},
|
||||
});
|
||||
const result = await spawn({ agentId: "research" });
|
||||
expectStatus(result, "accepted");
|
||||
expectStatus(result, "forbidden");
|
||||
expect(result.error ?? "").toBe(
|
||||
'agentId "research" is not in the configured agent registry (allowed: none)',
|
||||
);
|
||||
expect(hoisted.callGatewayMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -122,7 +122,10 @@ async function executeBoundAccountSpawn(params: {
|
||||
setSessionsSpawnConfigOverride({
|
||||
session: { mainKey: "main", scope: "per-sender" },
|
||||
messages: { queue: { debounceMs: 0 } },
|
||||
agents: { defaults: { subagents: { allowAgents: ["bot-alpha"] } } },
|
||||
agents: {
|
||||
defaults: { subagents: { allowAgents: ["bot-alpha"] } },
|
||||
list: [{ id: "main" }, { id: "bot-alpha" }],
|
||||
},
|
||||
bindings: params.bindings,
|
||||
});
|
||||
setupSessionsSpawnGatewayMock({
|
||||
|
||||
@@ -41,6 +41,7 @@ describe("subagent target policy", () => {
|
||||
targetAgentId: "task-manager",
|
||||
requestedAgentId: "task-manager",
|
||||
allowAgents: ["planner", "checker"],
|
||||
configuredAgentIds: ["task-manager", "planner", "checker"],
|
||||
});
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) {
|
||||
@@ -65,6 +66,35 @@ describe("subagent target policy", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("filters explicit allowlists to configured target ids", () => {
|
||||
expect(
|
||||
resolveSubagentAllowedTargetIds({
|
||||
requesterAgentId: "main",
|
||||
allowAgents: ["planner", "stale"],
|
||||
configuredAgentIds: ["main", "planner"],
|
||||
}),
|
||||
).toEqual({
|
||||
allowAny: false,
|
||||
allowedIds: ["planner"],
|
||||
});
|
||||
|
||||
const result = resolveSubagentTargetPolicy({
|
||||
requesterAgentId: "main",
|
||||
targetAgentId: "stale",
|
||||
requestedAgentId: "stale",
|
||||
allowAgents: ["planner", "stale"],
|
||||
configuredAgentIds: ["main", "planner"],
|
||||
});
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) {
|
||||
throw new Error("Expected target policy to reject stale explicit target");
|
||||
}
|
||||
expect(result.allowedText).toBe("planner");
|
||||
expect(result.error).toBe(
|
||||
'agentId "stale" is not in the configured agent registry (allowed: planner)',
|
||||
);
|
||||
});
|
||||
|
||||
it("limits wildcard allowlists to configured agents plus the requester", () => {
|
||||
expect(
|
||||
resolveSubagentAllowedTargetIds({
|
||||
@@ -97,7 +127,7 @@ describe("subagent target policy", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves explicit targets when wildcard allowlists are mixed", () => {
|
||||
it("filters explicit targets when wildcard allowlists are mixed", () => {
|
||||
expect(
|
||||
resolveSubagentAllowedTargetIds({
|
||||
requesterAgentId: "main",
|
||||
@@ -106,17 +136,22 @@ describe("subagent target policy", () => {
|
||||
}),
|
||||
).toEqual({
|
||||
allowAny: true,
|
||||
allowedIds: ["beta", "main", "planner"],
|
||||
allowedIds: ["main", "planner"],
|
||||
});
|
||||
|
||||
expect(
|
||||
resolveSubagentTargetPolicy({
|
||||
requesterAgentId: "main",
|
||||
targetAgentId: "beta",
|
||||
requestedAgentId: "beta",
|
||||
allowAgents: ["*", "beta"],
|
||||
configuredAgentIds: ["main", "planner"],
|
||||
}),
|
||||
).toEqual({ ok: true });
|
||||
const result = resolveSubagentTargetPolicy({
|
||||
requesterAgentId: "main",
|
||||
targetAgentId: "beta",
|
||||
requestedAgentId: "beta",
|
||||
allowAgents: ["*", "beta"],
|
||||
configuredAgentIds: ["main", "planner"],
|
||||
});
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) {
|
||||
throw new Error("Expected target policy to reject stale mixed explicit target");
|
||||
}
|
||||
expect(result.error).toBe(
|
||||
'agentId "beta" is not in the configured agent registry (allowed: main, planner)',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -26,6 +26,20 @@ function normalizeAllowAgents(allowAgents: readonly string[] | undefined): {
|
||||
};
|
||||
}
|
||||
|
||||
function normalizeConfiguredAgentIds(
|
||||
configuredAgentIds: readonly string[] | undefined,
|
||||
): Set<string> {
|
||||
return new Set((configuredAgentIds ?? []).map((id) => normalizeAgentId(id)).filter(Boolean));
|
||||
}
|
||||
|
||||
function filterConfiguredAllowedIds(params: {
|
||||
allowedIds: readonly string[];
|
||||
configuredAgentIds?: readonly string[];
|
||||
}): string[] {
|
||||
const configuredIds = normalizeConfiguredAgentIds(params.configuredAgentIds);
|
||||
return params.allowedIds.filter((id) => configuredIds.has(id));
|
||||
}
|
||||
|
||||
export function resolveSubagentAllowedTargetIds(params: {
|
||||
requesterAgentId: string;
|
||||
allowAgents?: readonly string[];
|
||||
@@ -40,10 +54,7 @@ export function resolveSubagentAllowedTargetIds(params: {
|
||||
};
|
||||
}
|
||||
if (policy.allowAny) {
|
||||
const configuredIds = (params.configuredAgentIds ?? [])
|
||||
.map((id) => normalizeAgentId(id))
|
||||
.filter(Boolean);
|
||||
configuredIds.push(...policy.allowedIds);
|
||||
const configuredIds = Array.from(normalizeConfiguredAgentIds(params.configuredAgentIds));
|
||||
if (requesterAgentId) {
|
||||
configuredIds.push(requesterAgentId);
|
||||
}
|
||||
@@ -54,7 +65,10 @@ export function resolveSubagentAllowedTargetIds(params: {
|
||||
}
|
||||
return {
|
||||
allowAny: false,
|
||||
allowedIds: policy.allowedIds,
|
||||
allowedIds: filterConfiguredAllowedIds({
|
||||
allowedIds: policy.allowedIds,
|
||||
configuredAgentIds: params.configuredAgentIds,
|
||||
}).toSorted((a, b) => a.localeCompare(b)),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -80,7 +94,8 @@ export function resolveSubagentTargetPolicy(params: {
|
||||
return { ok: true };
|
||||
}
|
||||
const allowedText = allowed.allowedIds.length > 0 ? allowed.allowedIds.join(", ") : "none";
|
||||
if (allowed.allowAny) {
|
||||
const policy = normalizeAllowAgents(params.allowAgents);
|
||||
if (allowed.allowAny || policy.allowedIds.includes(targetAgentId)) {
|
||||
return {
|
||||
ok: false,
|
||||
allowedText,
|
||||
|
||||
@@ -75,6 +75,32 @@ describe("agents_list tool", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("does not advertise stale allowlist-only targets as spawnable agents", async () => {
|
||||
loadConfigMock.mockReturnValue({
|
||||
agents: {
|
||||
list: [
|
||||
{
|
||||
id: "main",
|
||||
default: true,
|
||||
subagents: { allowAgents: ["stale"] },
|
||||
},
|
||||
],
|
||||
},
|
||||
} satisfies OpenClawConfig);
|
||||
|
||||
const result = await createAgentsListTool({ agentSessionKey: "agent:main:main" }).execute(
|
||||
"call",
|
||||
{},
|
||||
);
|
||||
const details = result.details as AgentListDetails;
|
||||
|
||||
expect(details).toStrictEqual({
|
||||
requester: "main",
|
||||
allowAny: false,
|
||||
agents: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("returns requester as the only target when no subagent allowlist is configured", async () => {
|
||||
loadConfigMock.mockReturnValue({
|
||||
agents: {
|
||||
@@ -103,6 +129,36 @@ describe("agents_list tool", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("uses the implicit default agent as a configured target", async () => {
|
||||
loadConfigMock.mockReturnValue({
|
||||
agents: {
|
||||
defaults: {
|
||||
subagents: { allowAgents: ["main"] },
|
||||
},
|
||||
},
|
||||
} satisfies OpenClawConfig);
|
||||
|
||||
const result = await createAgentsListTool({ agentSessionKey: "agent:main:main" }).execute(
|
||||
"call",
|
||||
{},
|
||||
);
|
||||
const details = result.details as AgentListDetails;
|
||||
|
||||
expect(details).toStrictEqual({
|
||||
requester: "main",
|
||||
allowAny: false,
|
||||
agents: [
|
||||
{
|
||||
id: "main",
|
||||
name: undefined,
|
||||
configured: true,
|
||||
model: undefined,
|
||||
agentRuntime: { id: "codex", source: "implicit" },
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores legacy env-forced plugin runtime selections", async () => {
|
||||
vi.stubEnv("OPENCLAW_AGENT_RUNTIME", "codex");
|
||||
loadConfigMock.mockReturnValue({
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
parseAgentSessionKey,
|
||||
} from "../../routing/session-key.js";
|
||||
import { resolveModelAgentRuntimeMetadata } from "../agent-runtime-metadata.js";
|
||||
import { listAgentIds } from "../agent-scope-config.js";
|
||||
import { resolveAgentConfig, resolveAgentEffectiveModelPrimary } from "../agent-scope.js";
|
||||
import { resolveDefaultModelForAgent } from "../model-selection.js";
|
||||
import { resolveSubagentAllowedTargetIds } from "../subagent-target-policy.js";
|
||||
@@ -58,7 +59,7 @@ export function createAgentsListTool(opts?: {
|
||||
cfg?.agents?.defaults?.subagents?.allowAgents;
|
||||
|
||||
const configuredAgents = Array.isArray(cfg.agents?.list) ? cfg.agents?.list : [];
|
||||
const configuredIds = configuredAgents.map((entry) => normalizeAgentId(entry.id));
|
||||
const configuredIds = listAgentIds(cfg);
|
||||
const configuredNameMap = new Map<string, string>();
|
||||
for (const entry of configuredAgents) {
|
||||
const name = entry?.name?.trim() ?? "";
|
||||
|
||||
@@ -26,6 +26,7 @@ import { maybeRepairOpenPolicyAllowFrom } from "./shared/open-policy-allowfrom.j
|
||||
import { cleanupLegacyPluginDependencyState } from "./shared/plugin-dependency-cleanup.js";
|
||||
import { repairStaleOAuthProfileShadows } from "./shared/stale-oauth-profile-shadows.js";
|
||||
import { maybeRepairStalePluginConfig } from "./shared/stale-plugin-config.js";
|
||||
import { maybeRepairStaleSubagentAllowlists } from "./shared/stale-subagent-allowlist.js";
|
||||
import { isUpdatePackageSwapInProgress } from "./shared/update-phase.js";
|
||||
|
||||
export async function runDoctorRepairSequence(params: {
|
||||
@@ -114,6 +115,7 @@ export async function runDoctorRepairSequence(params: {
|
||||
applyMutation(await maybeRepairAllowlistPolicyAllowFrom(state.candidate));
|
||||
applyMutation(maybeRepairOpenPolicyAllowFrom(state.candidate));
|
||||
applyMutation(maybeRepairGroupAllowFromFallback(state.candidate));
|
||||
applyMutation(maybeRepairStaleSubagentAllowlists(state.candidate));
|
||||
|
||||
const emptyAllowlistWarnings = scanEmptyAllowlistPolicyWarnings(state.candidate, {
|
||||
doctorFixCommand: params.doctorFixCommand,
|
||||
|
||||
@@ -47,6 +47,16 @@ function hasPluginLoadPaths(cfg: OpenClawConfig): boolean {
|
||||
return hasRecord(load) && Array.isArray(load.paths) && load.paths.length > 0;
|
||||
}
|
||||
|
||||
function hasSubagentAllowlistConfig(cfg: OpenClawConfig): boolean {
|
||||
if (Array.isArray(cfg.agents?.defaults?.subagents?.allowAgents)) {
|
||||
return true;
|
||||
}
|
||||
return listAgentRecords(cfg).some((agent) => {
|
||||
const subagents = hasRecord(agent.subagents) ? agent.subagents : undefined;
|
||||
return Array.isArray(subagents?.allowAgents);
|
||||
});
|
||||
}
|
||||
|
||||
function hasExplicitChannelPluginBlockerConfig(cfg: OpenClawConfig): boolean {
|
||||
if (cfg.plugins?.enabled === false) {
|
||||
return true;
|
||||
@@ -436,6 +446,19 @@ export async function collectDoctorPreviewWarnings(params: {
|
||||
const { collectCodexRouteWarnings } = await import("./codex-route-warnings.js");
|
||||
warnings.push(...collectCodexRouteWarnings({ cfg: params.cfg, env }));
|
||||
}
|
||||
if (hasSubagentAllowlistConfig(params.cfg)) {
|
||||
const { collectStaleSubagentAllowlistWarnings, scanStaleSubagentAllowlistReferences } =
|
||||
await import("./stale-subagent-allowlist.js");
|
||||
const staleSubagentAllowlistHits = scanStaleSubagentAllowlistReferences(params.cfg);
|
||||
if (staleSubagentAllowlistHits.length > 0) {
|
||||
warnings.push(
|
||||
collectStaleSubagentAllowlistWarnings({
|
||||
hits: staleSubagentAllowlistHits,
|
||||
doctorFixCommand: params.doctorFixCommand,
|
||||
}).join("\n"),
|
||||
);
|
||||
}
|
||||
}
|
||||
const { collectCodexNativeAssetWarnings } = await import("./codex-native-assets.js");
|
||||
warnings.push(...(await collectCodexNativeAssetWarnings({ cfg: params.cfg, env })));
|
||||
|
||||
|
||||
123
src/commands/doctor/shared/stale-subagent-allowlist.test.ts
Normal file
123
src/commands/doctor/shared/stale-subagent-allowlist.test.ts
Normal file
@@ -0,0 +1,123 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { OpenClawConfig } from "../../../config/types.openclaw.js";
|
||||
import {
|
||||
collectStaleSubagentAllowlistWarnings,
|
||||
maybeRepairStaleSubagentAllowlists,
|
||||
scanStaleSubagentAllowlistReferences,
|
||||
} from "./stale-subagent-allowlist.js";
|
||||
|
||||
describe("stale subagent allowlist doctor repair", () => {
|
||||
it("detects stale default and per-agent subagent targets", () => {
|
||||
const cfg = {
|
||||
agents: {
|
||||
defaults: {
|
||||
subagents: {
|
||||
allowAgents: ["planner", "stale-default"],
|
||||
},
|
||||
},
|
||||
list: [
|
||||
{
|
||||
id: "main",
|
||||
subagents: {
|
||||
allowAgents: ["planner", "stale-main"],
|
||||
},
|
||||
},
|
||||
{ id: "planner" },
|
||||
],
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
|
||||
expect(scanStaleSubagentAllowlistReferences(cfg)).toStrictEqual([
|
||||
{
|
||||
pathLabel: "agents.defaults.subagents.allowAgents",
|
||||
agentId: "stale-default",
|
||||
normalizedAgentId: "stale-default",
|
||||
},
|
||||
{
|
||||
pathLabel: "agents.list.0.subagents.allowAgents",
|
||||
agentId: "stale-main",
|
||||
normalizedAgentId: "stale-main",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("keeps wildcard, configured OpenClaw agents, and configured ACP targets", () => {
|
||||
const cfg = {
|
||||
acp: {
|
||||
defaultAgent: "claude",
|
||||
allowedAgents: ["codex"],
|
||||
},
|
||||
agents: {
|
||||
defaults: {
|
||||
subagents: {
|
||||
allowAgents: ["*", "main", "planner", "codex", "claude", "writer", "stale"],
|
||||
},
|
||||
},
|
||||
list: [
|
||||
{ id: "main" },
|
||||
{ id: "planner" },
|
||||
{
|
||||
id: "writer-agent",
|
||||
runtime: { type: "acp", acp: { agent: "writer" } },
|
||||
},
|
||||
],
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
|
||||
expect(scanStaleSubagentAllowlistReferences(cfg)).toStrictEqual([
|
||||
{
|
||||
pathLabel: "agents.defaults.subagents.allowAgents",
|
||||
agentId: "stale",
|
||||
normalizedAgentId: "stale",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("repairs stale entries without widening an explicit empty allowlist", () => {
|
||||
const cfg = {
|
||||
agents: {
|
||||
defaults: {
|
||||
subagents: {
|
||||
allowAgents: ["stale"],
|
||||
},
|
||||
},
|
||||
list: [
|
||||
{
|
||||
id: "main",
|
||||
subagents: {
|
||||
allowAgents: ["*", "planner", "stale-main"],
|
||||
},
|
||||
},
|
||||
{ id: "planner" },
|
||||
],
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
|
||||
const result = maybeRepairStaleSubagentAllowlists(cfg);
|
||||
|
||||
expect(result.config.agents?.defaults?.subagents?.allowAgents).toStrictEqual([]);
|
||||
expect(result.config.agents?.list?.[0]?.subagents?.allowAgents).toStrictEqual(["*", "planner"]);
|
||||
expect(result.changes).toStrictEqual([
|
||||
"- agents.defaults.subagents.allowAgents: removed 1 stale subagent target id (stale)",
|
||||
"- agents.list.0.subagents.allowAgents: removed 1 stale subagent target id (stale-main)",
|
||||
]);
|
||||
});
|
||||
|
||||
it("formats preview warnings with the doctor fix command", () => {
|
||||
const warnings = collectStaleSubagentAllowlistWarnings({
|
||||
hits: [
|
||||
{
|
||||
pathLabel: "agents.defaults.subagents.allowAgents",
|
||||
agentId: "research",
|
||||
normalizedAgentId: "research",
|
||||
},
|
||||
],
|
||||
doctorFixCommand: "openclaw doctor --fix",
|
||||
});
|
||||
|
||||
expect(warnings).toStrictEqual([
|
||||
'- agents.defaults.subagents.allowAgents: stale subagent target "research" is not in the configured agent registry.',
|
||||
'- Run "openclaw doctor --fix" to remove stale subagent target ids, or add a configured agent or ACP target for each intended target.',
|
||||
]);
|
||||
});
|
||||
});
|
||||
178
src/commands/doctor/shared/stale-subagent-allowlist.ts
Normal file
178
src/commands/doctor/shared/stale-subagent-allowlist.ts
Normal file
@@ -0,0 +1,178 @@
|
||||
import { listAgentIds } from "../../../agents/agent-scope-config.js";
|
||||
import type { OpenClawConfig } from "../../../config/types.openclaw.js";
|
||||
import { normalizeAgentId } from "../../../routing/session-key.js";
|
||||
import { normalizeOptionalString } from "../../../shared/string-coerce.js";
|
||||
|
||||
export type StaleSubagentAllowlistHit = {
|
||||
pathLabel: string;
|
||||
agentId: string;
|
||||
normalizedAgentId: string;
|
||||
};
|
||||
|
||||
function normalizeOptionalAgentId(value: string | undefined | null): string | undefined {
|
||||
const trimmed = normalizeOptionalString(value) ?? "";
|
||||
if (!trimmed) {
|
||||
return undefined;
|
||||
}
|
||||
return normalizeAgentId(trimmed);
|
||||
}
|
||||
|
||||
function collectConfiguredSubagentTargetIds(cfg: OpenClawConfig): Set<string> {
|
||||
const ids = new Set<string>(listAgentIds(cfg));
|
||||
for (const agent of cfg.agents?.list ?? []) {
|
||||
if (agent.runtime?.type !== "acp") {
|
||||
continue;
|
||||
}
|
||||
const acpAgent = normalizeOptionalAgentId(agent.runtime.acp?.agent);
|
||||
if (acpAgent) {
|
||||
ids.add(acpAgent);
|
||||
}
|
||||
}
|
||||
const defaultAcpAgent = normalizeOptionalAgentId(cfg.acp?.defaultAgent);
|
||||
if (defaultAcpAgent) {
|
||||
ids.add(defaultAcpAgent);
|
||||
}
|
||||
for (const entry of cfg.acp?.allowedAgents ?? []) {
|
||||
if (entry.trim() === "*") {
|
||||
continue;
|
||||
}
|
||||
const acpAgent = normalizeOptionalAgentId(entry);
|
||||
if (acpAgent) {
|
||||
ids.add(acpAgent);
|
||||
}
|
||||
}
|
||||
return ids;
|
||||
}
|
||||
|
||||
function collectStaleAllowlistEntries(params: {
|
||||
allowAgents: unknown;
|
||||
pathLabel: string;
|
||||
configuredTargetIds: ReadonlySet<string>;
|
||||
}): StaleSubagentAllowlistHit[] {
|
||||
if (!Array.isArray(params.allowAgents)) {
|
||||
return [];
|
||||
}
|
||||
const hits: StaleSubagentAllowlistHit[] = [];
|
||||
const seen = new Set<string>();
|
||||
for (const entry of params.allowAgents) {
|
||||
if (typeof entry !== "string") {
|
||||
continue;
|
||||
}
|
||||
const trimmed = entry.trim();
|
||||
if (!trimmed || trimmed === "*") {
|
||||
continue;
|
||||
}
|
||||
const normalizedAgentId = normalizeAgentId(trimmed);
|
||||
if (params.configuredTargetIds.has(normalizedAgentId)) {
|
||||
continue;
|
||||
}
|
||||
const key = `${params.pathLabel}:${normalizedAgentId}`;
|
||||
if (seen.has(key)) {
|
||||
continue;
|
||||
}
|
||||
seen.add(key);
|
||||
hits.push({
|
||||
pathLabel: params.pathLabel,
|
||||
agentId: trimmed,
|
||||
normalizedAgentId,
|
||||
});
|
||||
}
|
||||
return hits;
|
||||
}
|
||||
|
||||
export function scanStaleSubagentAllowlistReferences(
|
||||
cfg: OpenClawConfig,
|
||||
): StaleSubagentAllowlistHit[] {
|
||||
const configuredTargetIds = collectConfiguredSubagentTargetIds(cfg);
|
||||
const hits: StaleSubagentAllowlistHit[] = [];
|
||||
hits.push(
|
||||
...collectStaleAllowlistEntries({
|
||||
allowAgents: cfg.agents?.defaults?.subagents?.allowAgents,
|
||||
pathLabel: "agents.defaults.subagents.allowAgents",
|
||||
configuredTargetIds,
|
||||
}),
|
||||
);
|
||||
const agents = Array.isArray(cfg.agents?.list) ? cfg.agents.list : [];
|
||||
for (const [index, agent] of agents.entries()) {
|
||||
hits.push(
|
||||
...collectStaleAllowlistEntries({
|
||||
allowAgents: agent?.subagents?.allowAgents,
|
||||
pathLabel: `agents.list.${index}.subagents.allowAgents`,
|
||||
configuredTargetIds,
|
||||
}),
|
||||
);
|
||||
}
|
||||
return hits;
|
||||
}
|
||||
|
||||
export function collectStaleSubagentAllowlistWarnings(params: {
|
||||
hits: readonly StaleSubagentAllowlistHit[];
|
||||
doctorFixCommand: string;
|
||||
}): string[] {
|
||||
if (params.hits.length === 0) {
|
||||
return [];
|
||||
}
|
||||
return [
|
||||
...params.hits.map(
|
||||
(hit) =>
|
||||
`- ${hit.pathLabel}: stale subagent target "${hit.agentId}" is not in the configured agent registry.`,
|
||||
),
|
||||
`- Run "${params.doctorFixCommand}" to remove stale subagent target ids, or add a configured agent or ACP target for each intended target.`,
|
||||
];
|
||||
}
|
||||
|
||||
function filterAllowAgents(params: {
|
||||
allowAgents: string[];
|
||||
staleTargetIds: ReadonlySet<string>;
|
||||
}): string[] {
|
||||
return params.allowAgents.filter((entry) => {
|
||||
const trimmed = entry.trim();
|
||||
return !trimmed || trimmed === "*" || !params.staleTargetIds.has(normalizeAgentId(trimmed));
|
||||
});
|
||||
}
|
||||
|
||||
export function maybeRepairStaleSubagentAllowlists(cfg: OpenClawConfig): {
|
||||
config: OpenClawConfig;
|
||||
changes: string[];
|
||||
} {
|
||||
const hits = scanStaleSubagentAllowlistReferences(cfg);
|
||||
if (hits.length === 0) {
|
||||
return { config: cfg, changes: [] };
|
||||
}
|
||||
|
||||
const next = structuredClone(cfg);
|
||||
const hitsByPath = new Map<string, StaleSubagentAllowlistHit[]>();
|
||||
for (const hit of hits) {
|
||||
hitsByPath.set(hit.pathLabel, [...(hitsByPath.get(hit.pathLabel) ?? []), hit]);
|
||||
}
|
||||
|
||||
const defaultsHits = hitsByPath.get("agents.defaults.subagents.allowAgents") ?? [];
|
||||
if (defaultsHits.length > 0 && Array.isArray(next.agents?.defaults?.subagents?.allowAgents)) {
|
||||
const staleTargetIds = new Set(defaultsHits.map((hit) => hit.normalizedAgentId));
|
||||
next.agents.defaults.subagents.allowAgents = filterAllowAgents({
|
||||
allowAgents: next.agents.defaults.subagents.allowAgents,
|
||||
staleTargetIds,
|
||||
});
|
||||
}
|
||||
|
||||
const agents = Array.isArray(next.agents?.list) ? next.agents.list : [];
|
||||
for (const [index, agent] of agents.entries()) {
|
||||
const pathLabel = `agents.list.${index}.subagents.allowAgents`;
|
||||
const agentHits = hitsByPath.get(pathLabel) ?? [];
|
||||
if (agentHits.length === 0 || !Array.isArray(agent?.subagents?.allowAgents)) {
|
||||
continue;
|
||||
}
|
||||
const staleTargetIds = new Set(agentHits.map((hit) => hit.normalizedAgentId));
|
||||
agent.subagents.allowAgents = filterAllowAgents({
|
||||
allowAgents: agent.subagents.allowAgents,
|
||||
staleTargetIds,
|
||||
});
|
||||
}
|
||||
|
||||
const changes = [...hitsByPath.entries()].map(([pathLabel, pathHits]) => {
|
||||
const ids = pathHits.map((hit) => hit.agentId).join(", ");
|
||||
return `- ${pathLabel}: removed ${pathHits.length} stale subagent target id${pathHits.length === 1 ? "" : "s"} (${ids})`;
|
||||
});
|
||||
|
||||
return { config: next, changes };
|
||||
}
|
||||
Reference in New Issue
Block a user