Skip to content

Commit 89cd13e

Browse files
steipeteVectorPeak
steipete
and
VectorPeak
committed
fix(config): preserve identity avatar issue paths
Co-authored-by: VectorPeak <[email protected]>
1 parent 7ff597a commit 89cd13e

6 files changed

Lines changed: 29 additions & 145 deletions

File tree

‎src/cli/config-cli.test.ts‎

Lines changed: 6 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,16 +1095,10 @@ describe("config cli", () => {
10951095
setSnapshotOnce({
10961096
path: "/tmp/openclaw.json",
10971097
exists: true,
1098-
raw: '{"$include":"./agents.json"}',
1099-
parsed: { $include: "./agents.json" },
1100-
sourceConfig: {
1101-
agents: { list: [{ id: "main", name: "main" }] },
1102-
diagnostics: { otel: { headers: { "0": "value" } } },
1103-
},
1104-
resolved: {
1105-
agents: { list: [{ id: "main", name: "main" }] },
1106-
diagnostics: { otel: { headers: { "0": "value" } } },
1107-
},
1098+
raw: "{}",
1099+
parsed: {},
1100+
sourceConfig: {},
1101+
resolved: {},
11081102
valid: true,
11091103
runtimeConfig: {},
11101104
config: {},
@@ -1115,8 +1109,6 @@ describe("config cli", () => {
11151109
message:
11161110
'channels.mattermost.dmPolicy="open" but channels.mattermost.allowFrom does not include "*"; all DMs will be dropped.',
11171111
},
1118-
{ path: "agents.list.0.name", message: "agent name warning" },
1119-
{ path: "diagnostics.otel.headers.0", message: "numeric object key warning" },
11201112
],
11211113
legacyIssues: [],
11221114
});
@@ -1127,9 +1119,6 @@ describe("config cli", () => {
11271119
expect(mockError).not.toHaveBeenCalled();
11281120
expectLogIncludes("Config valid:");
11291121
expectLogIncludes("channels.mattermost.allowFrom");
1130-
expectLogIncludes("agents.list[0].name");
1131-
expectLogIncludes("diagnostics.otel.headers.0");
1132-
expectLogExcludes("diagnostics.otel.headers[0]");
11331122
expectLogIncludes("all DMs will be dropped");
11341123
});
11351124

@@ -1152,63 +1141,6 @@ describe("config cli", () => {
11521141
expect(mockLog).not.toHaveBeenCalled();
11531142
});
11541143

1155-
it("formats array indexes only at the human display boundary", async () => {
1156-
setSnapshotOnce(
1157-
makeInvalidSnapshot({
1158-
parsed: { $include: "./agents.json" },
1159-
sourceConfig: {
1160-
agents: {
1161-
list: [
1162-
{
1163-
id: "proof-agent",
1164-
identity: { avatar: "~/avatar.png" },
1165-
tools: { exec: { commandHighlighting: true } },
1166-
},
1167-
],
1168-
},
1169-
bindings: [
1170-
{
1171-
type: "acp",
1172-
agentId: "main",
1173-
match: { channel: "test" },
1174-
acp: { label: "claude" },
1175-
},
1176-
],
1177-
diagnostics: { otel: { headers: { "0": "value" } } },
1178-
},
1179-
issues: [
1180-
{
1181-
path: "agents.list.0.tools.exec.commandHighlighting",
1182-
message: "Expected boolean",
1183-
},
1184-
{
1185-
path: "bindings.0.acp",
1186-
message: 'Unrecognized key: "agent"',
1187-
},
1188-
{
1189-
path: "agents.list.0.identity.avatar",
1190-
message:
1191-
"identity.avatar must be a workspace-relative path, http(s) URL, or data URI.",
1192-
},
1193-
{
1194-
path: "diagnostics.otel.headers.0",
1195-
message: "Expected string",
1196-
},
1197-
],
1198-
}),
1199-
);
1200-
1201-
await expect(runConfigCommand(["config", "validate"])).rejects.toThrow("__exit__:1");
1202-
1203-
const output = mockError.mock.calls.map((call) => String(call[0])).join("\n");
1204-
expect(output).toContain("agents.list[0].tools.exec.commandHighlighting");
1205-
expect(output).toContain("bindings[0].acp");
1206-
expect(output).toContain("agents.list[0].identity.avatar");
1207-
expect(output).toContain("diagnostics.otel.headers.0");
1208-
expect(output).not.toContain("diagnostics.otel.headers[0]");
1209-
expect(mockLog).not.toHaveBeenCalled();
1210-
});
1211-
12121144
it("replaces doctor advice for plugin packaging compiled-output failures", async () => {
12131145
setSnapshotOnce(
12141146
makeInvalidSnapshot({
@@ -1283,21 +1215,14 @@ describe("config cli", () => {
12831215
it("returns machine-readable JSON with --json for invalid config", async () => {
12841216
setSnapshotOnce(
12851217
makeInvalidSnapshot({
1286-
parsed: { bindings: [{ acp: { agent: "claude" } }] },
1287-
issues: [
1288-
{ path: "gateway.bind", message: "Invalid enum value" },
1289-
{ path: "bindings.0.acp", message: 'Unrecognized key: "agent"' },
1290-
],
1218+
issues: [{ path: "gateway.bind", message: "Invalid enum value" }],
12911219
}),
12921220
);
12931221

12941222
const payload = await runValidateJsonAndGetPayload();
12951223
expect(payload.valid).toBe(false);
12961224
expect(payload.path).toBe("/tmp/custom-openclaw.json");
1297-
expect(payload.issues).toEqual([
1298-
{ path: "gateway.bind", message: "Invalid enum value" },
1299-
{ path: "bindings.0.acp", message: 'Unrecognized key: "agent"' },
1300-
]);
1225+
expect(payload.issues).toEqual([{ path: "gateway.bind", message: "Invalid enum value" }]);
13011226
expect(mockError).not.toHaveBeenCalled();
13021227
});
13031228

‎src/cli/config-cli.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2667,10 +2667,7 @@ async function runConfigValidate(opts: { json?: boolean; runtime?: RuntimeEnv }
26672667
runtime.log(success(`Config valid: ${shortPath}`));
26682668
if (warnings.length > 0) {
26692669
runtime.log(warn(`${warnings.length} warning(s):`));
2670-
for (const line of formatConfigIssueLines(warnings, warn("!"), {
2671-
normalizeRoot: true,
2672-
displayRoot: snapshot.sourceConfig,
2673-
})) {
2670+
for (const line of formatConfigIssueLines(warnings, warn("!"), { normalizeRoot: true })) {
26742671
runtime.log(` ${line}`);
26752672
}
26762673
}

‎src/config/config.identity-avatar.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ describe("identity avatar validation", () => {
4747
expect(res.ok).toBe(false);
4848
if (!res.ok) {
4949
expect(res.issues[0]?.path).toBe("agents.list.0.identity.avatar");
50+
expect(res.issues[0]?.pathSegments).toEqual(["agents", "list", 0, "identity", "avatar"]);
5051
}
5152
});
5253
});

‎src/config/issue-format.test.ts‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -55,36 +55,6 @@ describe("config issue format", () => {
5555
).toBe("- gateway.\\nbind: bad\\r\\n\\tvalue");
5656
});
5757

58-
it("formats numeric segments by traversing the display root", () => {
59-
const displayRoot = {
60-
agents: { list: [{ identity: { avatar: "~/avatar.png" } }] },
61-
bindings: [{ acp: { agent: "claude" } }],
62-
diagnostics: { otel: { headers: { "0": false } } },
63-
};
64-
65-
expect(
66-
formatConfigIssueLines(
67-
[
68-
{ path: "bindings.0.acp", message: "binding union issue" },
69-
{ path: "agents.list.0.identity.avatar", message: "manual issue" },
70-
{ path: "diagnostics.otel.headers.0", message: "numeric object key" },
71-
],
72-
"-",
73-
{ displayRoot },
74-
),
75-
).toEqual([
76-
"- bindings[0].acp: binding union issue",
77-
"- agents.list[0].identity.avatar: manual issue",
78-
"- diagnostics.otel.headers.0: numeric object key",
79-
]);
80-
});
81-
82-
it("keeps contract paths dotted when display context is absent", () => {
83-
const issue = { path: "agents.list.0.identity.avatar", message: "invalid" };
84-
85-
expect(formatConfigIssueLine(issue, "-")).toBe("- agents.list.0.identity.avatar: invalid");
86-
});
87-
8858
it("formats concise issue summaries", () => {
8959
expect(formatConfigIssueSummary([])).toBeNull();
9060
expect(

‎src/config/issue-format.ts‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ type ConfigIssueFormatOptions = {
1414
sourceFile?: string;
1515
};
1616

17-
type ConfigIssueSummaryOptions = Pick<ConfigIssueFormatOptions, "normalizeRoot"> & {
17+
type ConfigIssueSummaryOptions = ConfigIssueFormatOptions & {
1818
maxIssues?: number;
1919
};
2020

@@ -82,23 +82,6 @@ function resolveIssuePathForLine(
8282
return typeof path === "string" ? path : "";
8383
}
8484

85-
function formatIssuePathForDisplay(path: string, displayRoot: unknown): string {
86-
const segments = path.split(".").filter((segment) => segment.length > 0);
87-
let parent: unknown = displayRoot;
88-
return segments
89-
.map((segment, index) => {
90-
const number = Number(segment);
91-
const isArrayIndex =
92-
Array.isArray(parent) && /^(0|[1-9]\d*)$/.test(segment) && Number.isSafeInteger(number);
93-
parent =
94-
parent && typeof parent === "object"
95-
? (parent as Record<string, unknown>)[segment]
96-
: undefined;
97-
return isArrayIndex ? `[${segment}]` : `${index === 0 ? "" : "."}${segment}`;
98-
})
99-
.join("");
100-
}
101-
10285
/**
10386
* Format one config issue for terminal output.
10487
* Path and message are sanitized because issues can include user-edited config text.

‎src/config/validation.ts‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,11 @@ function isWorkspaceAvatarPath(value: string, workspaceDir: string): boolean {
961961
return isPathWithinRoot(workspaceRoot, resolved);
962962
}
963963

964+
function createIdentityAvatarIssue(index: number, message: string): ConfigValidationIssue {
965+
const pathSegments = ["agents", "list", index, "identity", "avatar"] as const;
966+
return withConfigIssuePath({ path: formatConfigPath(pathSegments), message }, pathSegments);
967+
}
968+
964969
function validateIdentityAvatar(config: OpenClawConfig): ConfigValidationIssue[] {
965970
const agents = config.agents?.list;
966971
if (!Array.isArray(agents) || agents.length === 0) {
@@ -983,29 +988,32 @@ function validateIdentityAvatar(config: OpenClawConfig): ConfigValidationIssue[]
983988
continue;
984989
}
985990
if (avatar.startsWith("~")) {
986-
issues.push({
987-
path: `agents.list.${index}.identity.avatar`,
988-
message: "identity.avatar must be a workspace-relative path, http(s) URL, or data URI.",
989-
});
991+
issues.push(
992+
createIdentityAvatarIssue(
993+
index,
994+
"identity.avatar must be a workspace-relative path, http(s) URL, or data URI.",
995+
),
996+
);
990997
continue;
991998
}
992999
const hasScheme = hasAvatarUriScheme(avatar);
9931000
if (hasScheme && !isWindowsAbsolutePath(avatar)) {
994-
issues.push({
995-
path: `agents.list.${index}.identity.avatar`,
996-
message: "identity.avatar must be a workspace-relative path, http(s) URL, or data URI.",
997-
});
1001+
issues.push(
1002+
createIdentityAvatarIssue(
1003+
index,
1004+
"identity.avatar must be a workspace-relative path, http(s) URL, or data URI.",
1005+
),
1006+
);
9981007
continue;
9991008
}
10001009
const workspaceDir = resolveAgentWorkspaceDir(
10011010
config,
10021011
entry.id ?? resolveDefaultAgentId(config),
10031012
);
10041013
if (!isWorkspaceAvatarPath(avatar, workspaceDir)) {
1005-
issues.push({
1006-
path: `agents.list.${index}.identity.avatar`,
1007-
message: "identity.avatar must stay within the agent workspace.",
1008-
});
1014+
issues.push(
1015+
createIdentityAvatarIssue(index, "identity.avatar must stay within the agent workspace."),
1016+
);
10091017
}
10101018
}
10111019
return issues;

0 commit comments

Comments
 (0)