Skip to content

Commit 2a7132a

Browse files
steipetesteipete
authored
refactor(agents): trim exec approval exports (#106401)
1 parent 194ed5d commit 2a7132a

9 files changed

Lines changed: 47 additions & 54 deletions

‎scripts/deadcode-exports.baseline.mjs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -873,10 +873,6 @@ export const KNIP_UNUSED_EXPORT_BASELINE = [
873873
"src/agents/auth-profiles/store.ts: testing",
874874
"src/agents/auth-profiles/usage.ts: testing",
875875
"src/agents/bash-process-registry.ts: resetProcessRegistryForTests",
876-
"src/agents/bash-tools.exec-approval-followup-state.ts: resetExecApprovalFollowupRuntimeHandoffsForTests",
877-
"src/agents/bash-tools.exec-approval-followup.ts: buildExecApprovalFollowupPrompt",
878-
"src/agents/bash-tools.exec-approval-request.ts: registerExecApprovalRequest",
879-
"src/agents/bash-tools.exec-approval-request.ts: registerExecApprovalRequestForHost",
880876
"src/agents/bash-tools.exec.ts: testing",
881877
"src/agents/bootstrap-cache.ts: clearAllBootstrapSnapshots",
882878
"src/agents/bootstrap-files.ts: resetBootstrapWarningCacheForTest",

‎src/agents/bash-tools.exec-approval-followup-state.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,3 @@ export function isExecApprovalFollowupSessionRebound(params: {
183183
const resolved = normalizeOptionalString(params.resolvedSessionId);
184184
return Boolean(expected && resolved && expected !== resolved);
185185
}
186-
187-
/** Clear exec approval follow-up handoffs between tests. */
188-
export function resetExecApprovalFollowupRuntimeHandoffsForTests(): void {
189-
execApprovalFollowupRuntimeHandoffs.clear();
190-
}

‎src/agents/bash-tools.exec-approval-followup.test.ts‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,7 @@ import {
2424
type DiagnosticEventPayload,
2525
} from "../infra/diagnostic-events.js";
2626
import { sendMessage } from "../infra/outbound/message.js";
27-
import {
28-
buildExecApprovalFollowupPrompt,
29-
sendExecApprovalFollowup,
30-
} from "./bash-tools.exec-approval-followup.js";
27+
import { sendExecApprovalFollowup } from "./bash-tools.exec-approval-followup.js";
3128
import { callGatewayTool } from "./tools/gateway.js";
3229

3330
const tempStoreDirs: string[] = [];
@@ -119,29 +116,43 @@ function expectDirectSend(expected: Record<string, unknown>) {
119116
}
120117

121118
describe("exec approval followup", () => {
122-
it("uses an explicit denial prompt when the command did not run", () => {
123-
const prompt = buildExecApprovalFollowupPrompt(
124-
"Exec denied (gateway id=req-1, user-denied): uname -a",
125-
);
119+
it("uses an explicit denial prompt when the command did not run", async () => {
120+
await sendExecApprovalFollowup({
121+
approvalId: "req-1",
122+
sessionKey: "agent:main:main",
123+
resultText: "Exec denied (gateway id=req-1, user-denied): uname -a",
124+
});
126125

126+
const prompt = expectGatewayAgentFollowup({ sessionKey: "agent:main:main" }).message;
127+
expect(prompt).toBeTypeOf("string");
127128
expect(prompt).toContain("did not run");
128129
expect(prompt).toContain("Do not mention, summarize, or reuse output");
129130
expect(prompt).not.toContain("already approved has completed");
130131
});
131132

132-
it("uses the denied followup branch for nested-parentheses denial metadata", () => {
133-
const prompt = buildExecApprovalFollowupPrompt(
134-
"Exec denied (gateway id=req-1, approval-timeout (allowlist-miss)): uname -a",
135-
);
133+
it("uses the denied followup branch for nested-parentheses denial metadata", async () => {
134+
await sendExecApprovalFollowup({
135+
approvalId: "req-1",
136+
sessionKey: "agent:main:main",
137+
resultText: "Exec denied (gateway id=req-1, approval-timeout (allowlist-miss)): uname -a",
138+
});
136139

140+
const prompt = expectGatewayAgentFollowup({ sessionKey: "agent:main:main" }).message;
141+
expect(prompt).toBeTypeOf("string");
137142
expect(prompt).toContain("did not run");
138143
expect(prompt).toContain("Do not mention, summarize, or reuse output");
139144
expect(prompt).not.toContain("already approved has completed");
140145
});
141146

142-
it("tells the agent to continue the task before replying when the command succeeds", () => {
143-
const prompt = buildExecApprovalFollowupPrompt("Exec finished (gateway id=req-1, code 0)\nok");
147+
it("tells the agent to continue the task before replying when the command succeeds", async () => {
148+
await sendExecApprovalFollowup({
149+
approvalId: "req-1",
150+
sessionKey: "agent:main:main",
151+
resultText: "Exec finished (gateway id=req-1, code 0)\nok",
152+
});
144153

154+
const prompt = expectGatewayAgentFollowup({ sessionKey: "agent:main:main" }).message;
155+
expect(prompt).toBeTypeOf("string");
145156
expect(prompt).toContain("continue from this result before replying to the user");
146157
expect(prompt).toContain("Continue the task if needed, then reply to the user");
147158
});

‎src/agents/bash-tools.exec-approval-followup.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ function formatUnknownError(error: unknown): string {
8484
}
8585

8686
/** Builds the prompt used to resume an agent after an approved async exec completes. */
87-
export function buildExecApprovalFollowupPrompt(resultText: string): string {
87+
function buildExecApprovalFollowupPrompt(resultText: string): string {
8888
const trimmed = resultText.trim();
8989
if (isExecDeniedResultText(trimmed)) {
9090
return buildExecDeniedFollowupPrompt(trimmed);

‎src/agents/bash-tools.exec-approval-request.test.ts‎

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ vi.mock("./tools/gateway.js", () => ({
3838
}));
3939

4040
let callGatewayTool: typeof import("./tools/gateway.js").callGatewayTool;
41-
let registerExecApprovalRequest: typeof import("./bash-tools.exec-approval-request.js").registerExecApprovalRequest;
42-
let registerExecApprovalRequestForHost: typeof import("./bash-tools.exec-approval-request.js").registerExecApprovalRequestForHost;
41+
let registerExecApprovalRequestForHostOrThrow: typeof import("./bash-tools.exec-approval-request.js").registerExecApprovalRequestForHostOrThrow;
4342

4443
const initialProcessPlatform = Object.getOwnPropertyDescriptor(process, "platform");
4544

@@ -75,7 +74,7 @@ function requireApprovalRequestPayload(callIndex: number): ApprovalRequestPayloa
7574
describe("exec approval requests", () => {
7675
beforeAll(async () => {
7776
({ callGatewayTool } = await import("./tools/gateway.js"));
78-
({ registerExecApprovalRequest, registerExecApprovalRequestForHost } =
77+
({ registerExecApprovalRequestForHostOrThrow } =
7978
await import("./bash-tools.exec-approval-request.js"));
8079
});
8180

@@ -100,10 +99,10 @@ describe("exec approval requests", () => {
10099

101100
try {
102101
await expect(
103-
registerExecApprovalRequest({
104-
id: "approval-id",
102+
registerExecApprovalRequestForHostOrThrow({
103+
approvalId: "approval-id",
105104
command: "echo hi",
106-
cwd: "/tmp",
105+
workdir: "/tmp",
107106
host: "gateway",
108107
security: "allowlist",
109108
ask: "on-miss",
@@ -124,10 +123,10 @@ describe("exec approval requests", () => {
124123

125124
try {
126125
await expect(
127-
registerExecApprovalRequest({
128-
id: "approval-id",
126+
registerExecApprovalRequestForHostOrThrow({
127+
approvalId: "approval-id",
129128
command: "echo hi",
130-
cwd: "/tmp",
129+
workdir: "/tmp",
131130
host: "gateway",
132131
security: "allowlist",
133132
ask: "on-miss",
@@ -141,7 +140,7 @@ describe("exec approval requests", () => {
141140
it("adds command spans to host approval registration payloads", async () => {
142141
vi.mocked(callGatewayTool).mockResolvedValue({ id: "approval-id", expiresAtMs: 1234 });
143142

144-
await registerExecApprovalRequestForHost({
143+
await registerExecApprovalRequestForHostOrThrow({
145144
approvalId: "approval-id",
146145
command: 'ls | grep "stuff" | python -c \'print("hi")\'',
147146
commandHighlighting: true,
@@ -163,7 +162,7 @@ describe("exec approval requests", () => {
163162
it("passes approval reviewer devices into host approval registration payloads", async () => {
164163
vi.mocked(callGatewayTool).mockResolvedValue({ id: "approval-id", expiresAtMs: 1234 });
165164

166-
await registerExecApprovalRequestForHost({
165+
await registerExecApprovalRequestForHostOrThrow({
167166
approvalId: "approval-id",
168167
command: "echo hi",
169168
approvalReviewerDeviceIds: ["device-ios-reviewer"],
@@ -180,7 +179,7 @@ describe("exec approval requests", () => {
180179
it("does not generate command spans by default", async () => {
181180
vi.mocked(callGatewayTool).mockResolvedValue({ id: "approval-id", expiresAtMs: 1234 });
182181

183-
await registerExecApprovalRequestForHost({
182+
await registerExecApprovalRequestForHostOrThrow({
184183
approvalId: "approval-id",
185184
command: 'ls | grep "stuff" | python -c \'print("hi")\'',
186185
workdir: "/tmp/project",
@@ -198,7 +197,7 @@ describe("exec approval requests", () => {
198197
it("does not generate command spans when command highlighting is disabled", async () => {
199198
vi.mocked(callGatewayTool).mockResolvedValue({ id: "approval-id", expiresAtMs: 1234 });
200199

201-
await registerExecApprovalRequestForHost({
200+
await registerExecApprovalRequestForHostOrThrow({
202201
approvalId: "approval-id",
203202
command: 'ls | grep "stuff" | python -c \'print("hi")\'',
204203
commandHighlighting: false,
@@ -217,7 +216,7 @@ describe("exec approval requests", () => {
217216
it("uses system run plan command text for host approval explanations", async () => {
218217
vi.mocked(callGatewayTool).mockResolvedValue({ id: "approval-id", expiresAtMs: 1234 });
219218

220-
await registerExecApprovalRequestForHost({
219+
await registerExecApprovalRequestForHostOrThrow({
221220
approvalId: "approval-id",
222221
systemRunPlan: {
223222
argv: ["node", "-e", "console.log(1)"],
@@ -240,15 +239,15 @@ describe("exec approval requests", () => {
240239
it("omits generated command spans for unsupported shell wrapper languages", async () => {
241240
vi.mocked(callGatewayTool).mockResolvedValue({ id: "approval-id", expiresAtMs: 1234 });
242241

243-
await registerExecApprovalRequestForHost({
242+
await registerExecApprovalRequestForHostOrThrow({
244243
approvalId: "approval-id-powershell",
245244
command: 'pwsh -Command "Get-ChildItem"',
246245
workdir: "/tmp/project",
247246
host: "node",
248247
security: "allowlist",
249248
ask: "always",
250249
});
251-
await registerExecApprovalRequestForHost({
250+
await registerExecApprovalRequestForHostOrThrow({
252251
approvalId: "approval-id-cmd",
253252
command: 'cmd.exe /d /s /c "dir"',
254253
workdir: "/tmp/project",
@@ -266,7 +265,7 @@ describe("exec approval requests", () => {
266265
setProcessPlatformForTest("win32");
267266
vi.mocked(callGatewayTool).mockResolvedValue({ id: "approval-id", expiresAtMs: 1234 });
268267

269-
await registerExecApprovalRequestForHost({
268+
await registerExecApprovalRequestForHostOrThrow({
270269
approvalId: "approval-id-powershell",
271270
command:
272271
'Set-Content -Path "windows-agent-proof.txt" -Value "WINDOWS_AGENT_EXEC_OK" -NoNewline',
@@ -284,7 +283,7 @@ describe("exec approval requests", () => {
284283
it("omits generated command spans for unsupported shell wrappers through system run carriers", async () => {
285284
vi.mocked(callGatewayTool).mockResolvedValue({ id: "approval-id", expiresAtMs: 1234 });
286285

287-
await registerExecApprovalRequestForHost({
286+
await registerExecApprovalRequestForHostOrThrow({
288287
approvalId: "approval-id-carrier",
289288
systemRunPlan: {
290289
argv: ["timeout", "5", "pwsh", "-Command", "Get-ChildItem"],
@@ -307,7 +306,7 @@ describe("exec approval requests", () => {
307306
it("keeps explicit command spans", async () => {
308307
vi.mocked(callGatewayTool).mockResolvedValue({ id: "approval-id", expiresAtMs: 1234 });
309308

310-
await registerExecApprovalRequestForHost({
309+
await registerExecApprovalRequestForHostOrThrow({
311310
approvalId: "approval-id",
312311
command: "echo hi",
313312
commandSpans: [{ startIndex: 0, endIndex: 4 }],

‎src/agents/bash-tools.exec-approval-request.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ export type ExecApprovalRegistration = {
135135
};
136136

137137
/** Registers a two-phase exec approval request with the gateway. */
138-
export async function registerExecApprovalRequest(
138+
async function registerExecApprovalRequest(
139139
params: RequestExecApprovalDecisionParams,
140140
): Promise<ExecApprovalRegistration> {
141141
// Two-phase registration is critical: the ID must be registered server-side
@@ -318,7 +318,7 @@ async function buildHostApprovalDecisionParams(
318318
}
319319

320320
/** Registers a host/node approval request without waiting for a decision. */
321-
export async function registerExecApprovalRequestForHost(
321+
async function registerExecApprovalRequestForHost(
322322
params: HostExecApprovalParams,
323323
): Promise<ExecApprovalRegistration> {
324324
return await registerExecApprovalRequest(await buildHostApprovalDecisionParams(params));

‎src/agents/bash-tools.exec-host-shared.test.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import {
88
consumeExecApprovalFollowupRuntimeHandoff,
99
isExecApprovalFollowupSessionRebound,
1010
registerExecApprovalFollowupRuntimeHandoff,
11-
resetExecApprovalFollowupRuntimeHandoffsForTests,
1211
} from "./bash-tools.exec-approval-followup-state.js";
1312
import {
1413
buildExecApprovalPendingToolResult,
@@ -72,7 +71,6 @@ describe("sendExecApprovalFollowupResult", () => {
7271
file: { version: 1, agents: {} },
7372
hash: "approvals-hash",
7473
});
75-
resetExecApprovalFollowupRuntimeHandoffsForTests();
7674
});
7775

7876
function firstExecApprovalFollowupCall():

‎src/agents/bash-tools.test.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ vi.mock("../utils/delivery-context.js", () => ({
8181
}));
8282

8383
vi.mock("./bash-tools.exec-approval-followup.js", () => ({
84-
buildExecApprovalFollowupPrompt: (text: string) => text,
8584
sendExecApprovalFollowup: vi.fn(async () => false),
8685
}));
8786

‎src/gateway/server-methods/agent.test.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,7 @@ import { expectDefined } from "@openclaw/normalization-core";
77
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
88
import { ErrorCodes } from "../../../packages/gateway-protocol/src/index.js";
99
import type { readAcpSessionMeta } from "../../acp/runtime/session-meta.js";
10-
import {
11-
registerExecApprovalFollowupRuntimeHandoff,
12-
resetExecApprovalFollowupRuntimeHandoffsForTests,
13-
} from "../../agents/bash-tools.exec-approval-followup-state.js";
10+
import { registerExecApprovalFollowupRuntimeHandoff } from "../../agents/bash-tools.exec-approval-followup-state.js";
1411
import type { AgentInternalEvent } from "../../agents/internal-events.js";
1512
import {
1613
createAgentRunRestartAbortError,
@@ -966,7 +963,6 @@ describe("gateway agent handler", () => {
966963
mocks.lifecycleGeneration = "test-generation";
967964
dateOnlyFakeClockActive = false;
968965
vi.useRealTimers();
969-
resetExecApprovalFollowupRuntimeHandoffsForTests();
970966
});
971967

972968
it("passes resolved maintenance config to the gateway admission store write", async () => {
@@ -9004,7 +9000,6 @@ describe("gateway agent handler chat.abort integration", () => {
90049000
mocks.lifecycleGeneration = "test-generation";
90059001
dateOnlyFakeClockActive = false;
90069002
vi.useRealTimers();
9007-
resetExecApprovalFollowupRuntimeHandoffsForTests();
90089003
}
90099004

90109005
beforeEach(() => {

0 commit comments

Comments
 (0)