negodiy/openclaw
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(tool-display): cd ~/dir && npm install shows as run cd — compound commands truncated to first stage (#21925)

Browse Source 
Merged via /review-pr -> /prepare-pr -> /merge-pr.

Prepared head SHA: 4728bfe8e75dfcdf21f9ac22e7a26d081dc95d93
Co-authored-by: Lukavyi <1013690+Lukavyi@users.noreply.github.com>
Co-authored-by: obviyus <22031114+obviyus@users.noreply.github.com>
Reviewed-by: @obviyus
This commit is contained in:
Taras Lukavyi
2026-02-21 03:33:32 +01:00
committed by GitHub
parent 866b33e0d3
commit 0e068194ad
3 changed files with 348 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -26,6 +26,7 @@ Docs: https://docs.openclaw.ai
- Gateway/Auth: require `gateway.trustedProxies` to include a loopback proxy address when `auth.mode="trusted-proxy"` and `bind="loopback"`, preventing same-host proxy misconfiguration from silently blocking auth. (#22082, follow-up to #20097) thanks @mbelinky.
- Security/OpenClawKit/UI: prevent injected inbound user context metadata blocks from leaking into chat history in TUI, webchat, and macOS surfaces by stripping all untrusted metadata prefixes at display boundaries. (#22142) Thanks @Mellowambience, @vincentkoc.
- Agents/System Prompt: label allowlisted senders as authorized senders to avoid implying ownership. Thanks @thewilloftheshadow.
- Agents/Tool display: fix exec cwd suffix inference so `pushd ... && popd ... && <command>` does not keep stale `(in <dir>)` context in summaries. (#21925) thanks @Lukavyi.
- Gateway/Auth: allow trusted-proxy mode with loopback bind for same-host reverse-proxy deployments, while still requiring configured `gateway.trustedProxies`. (#20097) thanks @xinhuagu.
- Gateway/Auth: allow authenticated clients across roles/scopes to call `health` while preserving role and scope enforcement for non-health methods. (#19699) thanks @Nachx639.
- Gateway/Security: remove shared-IP fallback for canvas endpoints and require token or session capability for canvas access. Thanks @thewilloftheshadow.

224
src/agents/tool-display-common.ts
View File

@@ -520,20 +520,26 @@ function scanTopLevelChars(
}
}
function firstTopLevelStage(command: string): string {
let splitIndex = -1;
function splitTopLevelStages(command: string): string[] {
const parts: string[] = [];
let start = 0;
scanTopLevelChars(command, (char, index) => {
if (char === ";") {
splitIndex = index;
return false;
parts.push(command.slice(start, index));
start = index + 1;
return true;
}
if ((char === "&" || char === "|") && command[index + 1] === char) {
splitIndex = index;
return false;
parts.push(command.slice(start, index));
start = index + 2;
return true;
}
return true;
});
return splitIndex >= 0 ? command.slice(0, splitIndex) : command;
parts.push(command.slice(start));
return parts.map((part) => part.trim()).filter((part) => part.length > 0);
}
function splitTopLevelPipes(command: string): string[] {
@@ -552,38 +558,79 @@ function splitTopLevelPipes(command: string): string[] {
return parts.map((part) => part.trim()).filter((part) => part.length > 0);
}
function stripShellPreamble(command: string): string {
function parseChdirTarget(head: string): string | undefined {
const words = splitShellWords(head, 3);
const bin = binaryName(words[0]);
if (bin === "cd" || bin === "pushd") {
return words[1] || undefined;
}
return undefined;
}
function isChdirCommand(head: string): boolean {
const bin = binaryName(splitShellWords(head, 2)[0]);
return bin === "cd" || bin === "pushd" || bin === "popd";
}
function isPopdCommand(head: string): boolean {
return binaryName(splitShellWords(head, 2)[0]) === "popd";
}
type PreambleResult = {
command: string;
chdirPath?: string;
};
function stripShellPreamble(command: string): PreambleResult {
let rest = command.trim();
let chdirPath: string | undefined;
for (let i = 0; i < 4; i += 1) {
const andIndex = rest.indexOf("&&");
const semicolonIndex = rest.indexOf(";");
const newlineIndex = rest.indexOf("\n");
const candidates = [
{ index: andIndex, length: 2 },
{ index: semicolonIndex, length: 1 },
{ index: newlineIndex, length: 1 },
]
.filter((candidate) => candidate.index >= 0)
.toSorted((a, b) => a.index - b.index);
const first = candidates[0];
// Find the first top-level separator (&&, ||, ;, \n) respecting quotes/escaping.
let first: { index: number; length: number; isOr?: boolean } | undefined;
scanTopLevelChars(rest, (char, idx) => {
if (char === "&" && rest[idx + 1] === "&") {
first = { index: idx, length: 2 };
return false;
}
if (char === "|" && rest[idx + 1] === "|") {
first = { index: idx, length: 2, isOr: true };
return false;
}
if (char === ";" || char === "\n") {
first = { index: idx, length: 1 };
return false;
}
});
const head = (first ? rest.slice(0, first.index) : rest).trim();
// cd/pushd/popd is preamble when followed by && / ; / \n, or when we already
// stripped at least one preamble segment (handles chained cd's like `cd /tmp && cd /app`).
// NOT for || — `cd /app || npm install` means npm runs when cd *fails*, so (in /app) is wrong.
const isChdir = (first ? !first.isOr : i > 0) && isChdirCommand(head);
const isPreamble =
head.startsWith("set ") || head.startsWith("export ") || head.startsWith("unset ");
head.startsWith("set ") || head.startsWith("export ") || head.startsWith("unset ") || isChdir;
if (!isPreamble) {
break;
}
if (isChdir) {
// popd returns to the previous directory, so inferred cwd from earlier
// preamble steps is no longer reliable.
if (isPopdCommand(head)) {
chdirPath = undefined;
} else {
chdirPath = parseChdirTarget(head) ?? chdirPath;
}
}
rest = first ? rest.slice(first.index + first.length).trimStart() : "";
if (!rest) {
break;
}
}
return rest.trim();
return { command: rest.trim(), chdirPath };
}
function summarizeKnownExec(words: string[]): string {
@@ -853,13 +900,7 @@ function summarizeKnownExec(words: string[]): string {
return /^[A-Za-z0-9._/-]+$/.test(arg) ? `run ${bin} ${arg}` : `run ${bin}`;
}
function summarizeExecCommand(command: string): string | undefined {
const cleaned = stripShellPreamble(command);
const stage = firstTopLevelStage(cleaned).trim();
if (!stage) {
return cleaned ? summarizeKnownExec(trimLeadingEnv(splitShellWords(cleaned))) : undefined;
}
function summarizePipeline(stage: string): string {
const pipeline = splitTopLevelPipes(stage);
if (pipeline.length > 1) {
const first = summarizeKnownExec(trimLeadingEnv(splitShellWords(pipeline[0])));
@@ -867,10 +908,108 @@ function summarizeExecCommand(command: string): string | undefined {
const extra = pipeline.length > 2 ? ` (+${pipeline.length - 2} steps)` : "";
return `${first} -> ${last}${extra}`;
}
return summarizeKnownExec(trimLeadingEnv(splitShellWords(stage)));
}
type ExecSummary = {
text: string;
chdirPath?: string;
allGeneric?: boolean;
};
function summarizeExecCommand(command: string): ExecSummary | undefined {
const { command: cleaned, chdirPath } = stripShellPreamble(command);
if (!cleaned) {
// All segments were preamble (e.g. `cd /tmp && cd /app`) — preserve chdirPath for context.
return chdirPath ? { text: "", chdirPath } : undefined;
}
const stages = splitTopLevelStages(cleaned);
if (stages.length === 0) {
return undefined;
}
const summaries = stages.map((stage) => summarizePipeline(stage));
const text = summaries.length === 1 ? summaries[0] : summaries.join(" → ");
const allGeneric = summaries.every((s) => isGenericSummary(s));
return { text, chdirPath, allGeneric };
}
/** Known summarizer prefixes that indicate a recognized command with useful context. */
const KNOWN_SUMMARY_PREFIXES = [
"check git",
"view git",
"show git",
"list git",
"switch git",
"create git",
"pull git",
"push git",
"fetch git",
"merge git",
"rebase git",
"stage git",
"restore git",
"reset git",
"stash git",
"search ",
"find files",
"list files",
"show first",
"show last",
"print line",
"print text",
"copy ",
"move ",
"remove ",
"create folder",
"create file",
"fetch http",
"install dependencies",
"run tests",
"run build",
"start app",
"run lint",
"run openclaw",
"run node script",
"run node ",
"run python",
"run ruby",
"run php",
"run sed",
"run git ",
"run npm ",
"run pnpm ",
"run yarn ",
"run bun ",
"check js syntax",
];
/** True when the summary is generic and the raw command would be more informative. */
function isGenericSummary(summary: string): boolean {
if (summary === "run command") {
return true;
}
// "run <binary>" or "run <binary> <arg>" without useful context
if (summary.startsWith("run ")) {
return !KNOWN_SUMMARY_PREFIXES.some((prefix) => summary.startsWith(prefix));
}
return false;
}
/** Compact the raw command for display: collapse whitespace, trim long strings. */
function compactRawCommand(raw: string, maxLength = 120): string {
const oneLine = raw
.replace(/\s*\n\s*/g, " ")
.replace(/\s{2,}/g, " ")
.trim();
if (oneLine.length <= maxLength) {
return oneLine;
}
return `${oneLine.slice(0, Math.max(0, maxLength - 1))}…`;
}
export function resolveExecDetail(args: unknown): string | undefined {
const record = asRecord(args);
if (!record) {
@@ -883,7 +1022,8 @@ export function resolveExecDetail(args: unknown): string | undefined {
}
const unwrapped = unwrapShellWrapper(raw);
const summary = summarizeExecCommand(unwrapped) ?? summarizeExecCommand(raw) ?? "run command";
const result = summarizeExecCommand(unwrapped) ?? summarizeExecCommand(raw);
const summary = result?.text || "run command";
const cwdRaw =
typeof record.workdir === "string"
@@ -891,9 +1031,25 @@ export function resolveExecDetail(args: unknown): string | undefined {
: typeof record.cwd === "string"
? record.cwd
: undefined;
const cwd = cwdRaw?.trim();
// Explicit workdir takes priority; fall back to cd path extracted from the command.
const cwd = cwdRaw?.trim() || result?.chdirPath || undefined;
return cwd ? `${summary} (in ${cwd})` : summary;
const compact = compactRawCommand(unwrapped);
// When ALL stages are generic (e.g. "run jj"), use the compact raw command instead.
// For mixed stages like "run cargo build → run tests", keep the summary since some parts are useful.
if (result?.allGeneric !== false && isGenericSummary(summary)) {
return cwd ? `${compact} (in ${cwd})` : compact;
}
const displaySummary = cwd ? `${summary} (in ${cwd})` : summary;
// Append the raw command when the summary differs meaningfully from the command itself.
if (compact && compact !== displaySummary && compact !== summary) {
return `${displaySummary}\n\n\`${compact}\``;
}
return displaySummary;
}
export function resolveActionSpec(

157
src/agents/tool-display.e2e.test.ts
View File

@@ -104,6 +104,163 @@ describe("tool display details", () => {
expect(detail).toContain(".openclaw/workspace)");
});
it("moves cd path to context suffix and appends raw command", () => {
const detail = formatToolDetail(
resolveToolDisplay({
name: "exec",
args: { command: "cd ~/my-project && npm install" },
}),
);
expect(detail).toBe(
"install dependencies (in ~/my-project)\n\n`cd ~/my-project && npm install`",
);
});
it("moves cd path to context suffix with multiple stages and raw command", () => {
const detail = formatToolDetail(
resolveToolDisplay({
name: "exec",
args: { command: "cd ~/my-project && npm install && npm test" },
}),
);
expect(detail).toBe(
"install dependencies → run tests (in ~/my-project)\n\n`cd ~/my-project && npm install && npm test`",
);
});
it("moves pushd path to context suffix and appends raw command", () => {
const detail = formatToolDetail(
resolveToolDisplay({
name: "exec",
args: { command: "pushd /tmp && git status" },
}),
);
expect(detail).toBe("check git status (in /tmp)\n\n`pushd /tmp && git status`");
});
it("clears inferred cwd when popd is stripped from preamble", () => {
const detail = formatToolDetail(
resolveToolDisplay({
name: "exec",
args: { command: "pushd /tmp && popd && npm install" },
}),
);
expect(detail).toBe("install dependencies\n\n`pushd /tmp && popd && npm install`");
});
it("moves cd path to context suffix with || separator", () => {
const detail = formatToolDetail(
resolveToolDisplay({
name: "exec",
args: { command: "cd /app || npm install" },
}),
);
// || means npm install runs when cd FAILS — cd should NOT be stripped as preamble.
// Both stages are summarized; cd is not treated as context prefix.
expect(detail).toMatch(/^run cd \/app → install dependencies/);
});
it("explicit workdir takes priority over cd path", () => {
const detail = formatToolDetail(
resolveToolDisplay({
name: "exec",
args: { command: "cd /tmp && npm install", workdir: "/app" },
}),
);
expect(detail).toBe("install dependencies (in /app)\n\n`cd /tmp && npm install`");
});
it("summarizes all stages and appends raw command", () => {
const detail = formatToolDetail(
resolveToolDisplay({
name: "exec",
args: { command: "git fetch && git rebase origin/main" },
}),
);
expect(detail).toBe(
"fetch git changes → rebase git branch\n\n`git fetch && git rebase origin/main`",
);
});
it("falls back to raw command for unknown binaries", () => {
const detail = formatToolDetail(
resolveToolDisplay({
name: "exec",
args: { command: "jj rebase -s abc -d main" },
}),
);
expect(detail).toBe("jj rebase -s abc -d main");
});
it("falls back to raw command for unknown binary with cwd", () => {
const detail = formatToolDetail(
resolveToolDisplay({
name: "exec",
args: { command: "mycli deploy --prod", workdir: "/app" },
}),
);
expect(detail).toBe("mycli deploy --prod (in /app)");
});
it("keeps multi-stage summary when only some stages are generic", () => {
const detail = formatToolDetail(
resolveToolDisplay({
name: "exec",
args: { command: "cargo build && npm test" },
}),
);
// "run cargo build" is generic, but "run tests" is known — keep joined summary
expect(detail).toMatch(/^run cargo build → run tests/);
});
it("handles standalone cd as raw command", () => {
const detail = formatToolDetail(
resolveToolDisplay({
name: "exec",
args: { command: "cd /tmp" },
}),
);
// standalone cd (no following command) — treated as raw since it's generic
expect(detail).toBe("cd /tmp");
});
it("handles chained cd commands using last path", () => {
const detail = formatToolDetail(
resolveToolDisplay({
name: "exec",
args: { command: "cd /tmp && cd /app" },
}),
);
// both cd's are preamble; last path wins
expect(detail).toBe("cd /tmp && cd /app (in /app)");
});
it("respects quotes when splitting preamble separators", () => {
const detail = formatToolDetail(
resolveToolDisplay({
name: "exec",
args: { command: 'export MSG="foo && bar" && echo test' },
}),
);
// The && inside quotes must not be treated as a separator —
// summary line should be "print text", not "run export" (which would happen
// if the quoted && was mistaken for a real separator).
expect(detail).toMatch(/^print text/);
});
it("recognizes heredoc/inline script exec details", () => {
const pyDetail = formatToolDetail(
resolveToolDisplay({