Skip to content

Commit f0dd4bf

Browse files
committed
ship: align action audit with main
1 parent 0790fb9 commit f0dd4bf

2 files changed

Lines changed: 3 additions & 118 deletions

File tree

apps/ade-cli/src/adeRpcServer.test.ts

Lines changed: 2 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2897,8 +2897,8 @@ describe("adeRpcServer", () => {
28972897
});
28982898

28992899
it("rejects run_ade_action when the action is not a callable on the domain service", async () => {
2900-
const { runtime, operationStart, operationFinish } = createRuntime();
2901-
const handler = createAdeRpcRequestHandler({ runtime, serverVersion: "test" });
2900+
const fixture = createRuntime();
2901+
const handler = createAdeRpcRequestHandler({ runtime: fixture.runtime, serverVersion: "test" });
29022902
await initialize(handler, { callerId: "agent-1", role: "agent" });
29032903

29042904
const response = await callTool(handler, "run_ade_action", {
@@ -2911,11 +2911,6 @@ describe("adeRpcServer", () => {
29112911
expect(JSON.stringify(response.error ?? response.structuredContent ?? {})).toContain(
29122912
"Action 'git.nonexistent_action' is not callable.",
29132913
);
2914-
expect(operationStart).toHaveBeenCalledTimes(1);
2915-
expect(operationFinish).toHaveBeenCalledWith(expect.objectContaining({
2916-
status: "failed",
2917-
metadataPatch: expect.objectContaining({ resultStatus: "error" }),
2918-
}));
29192914
});
29202915

29212916
it("reads ADE action status snapshots across operation/test/chat/pr", async () => {
@@ -3059,45 +3054,6 @@ describe("adeRpcServer", () => {
30593054
expect(operationFinish).not.toHaveBeenCalled();
30603055
});
30613056

3062-
it("does not mask successful action responses when audit finish fails", async () => {
3063-
const { runtime, operationFinish } = createRuntime();
3064-
operationFinish.mockImplementationOnce(() => {
3065-
throw new Error("finish failed");
3066-
});
3067-
const handler = createAdeRpcRequestHandler({ runtime, serverVersion: "test" });
3068-
3069-
await initialize(handler);
3070-
const response = await callTool(handler, "list_lanes", {});
3071-
3072-
expect(response.isError).toBeUndefined();
3073-
expect(response.structuredContent.lanes).toHaveLength(2);
3074-
expect(operationFinish).toHaveBeenCalledTimes(1);
3075-
expect(runtime.logger.warn).toHaveBeenCalledWith(
3076-
"ade_rpc.action_audit_finish_failed",
3077-
expect.objectContaining({ actionName: "list_lanes", status: "succeeded" }),
3078-
);
3079-
});
3080-
3081-
it("records failed audit metadata for returned action failure payloads", async () => {
3082-
const { runtime, operationFinish } = createRuntime();
3083-
runtime.gitService.push.mockResolvedValueOnce({ success: false, error: "push failed" });
3084-
const handler = createAdeRpcRequestHandler({ runtime, serverVersion: "test" });
3085-
3086-
await initialize(handler, { callerId: "agent-1", role: "agent" });
3087-
const response = await callTool(handler, "run_ade_action", {
3088-
domain: "git",
3089-
action: "push",
3090-
args: { laneId: "lane-1" },
3091-
});
3092-
3093-
expect(response.isError).toBeUndefined();
3094-
expect(response.structuredContent.result.success).toBe(false);
3095-
expect(operationFinish).toHaveBeenCalledWith(expect.objectContaining({
3096-
status: "failed",
3097-
metadataPatch: expect.objectContaining({ resultStatus: "error", error: "push failed" }),
3098-
}));
3099-
});
3100-
31013057
// ---------- Rate limit tests ----------
31023058

31033059
afterEach(() => {

apps/ade-cli/src/adeRpcServer.ts

Lines changed: 1 addition & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -5245,77 +5245,6 @@ export function createAdeRpcRequestHandler(args: {
52455245
throw new JsonRpcError(JsonRpcErrorCode.methodNotFound, `Unsupported ADE action: ${actionName}`);
52465246
};
52475247

5248-
const failedActionPayloadMessage = (value: unknown): string | null => {
5249-
if (!isRecord(value)) return null;
5250-
const failed = value.ok === false || value.success === false;
5251-
if (!failed) return null;
5252-
const error = value.error;
5253-
if (typeof error === "string" && error.trim()) return error.trim();
5254-
if (isRecord(error) && typeof error.message === "string" && error.message.trim()) {
5255-
return error.message.trim();
5256-
}
5257-
return "Action returned a failed result.";
5258-
};
5259-
5260-
const actionResultFailureMessage = (value: unknown): string | null => {
5261-
if (!isRecord(value)) return null;
5262-
return failedActionPayloadMessage(value) ?? failedActionPayloadMessage(value.result);
5263-
};
5264-
5265-
const finishActionAudit = (
5266-
actionName: string,
5267-
operationId: string,
5268-
status: "succeeded" | "failed",
5269-
metadataPatch: Record<string, unknown>,
5270-
): void => {
5271-
try {
5272-
runtime.operationService.finish({ operationId, status, metadataPatch });
5273-
} catch (error) {
5274-
runtime.logger.warn("ade_rpc.action_audit_finish_failed", {
5275-
actionName,
5276-
operationId,
5277-
status,
5278-
error: error instanceof Error ? error.message : String(error),
5279-
});
5280-
}
5281-
};
5282-
5283-
const callActionWithAudit = async (
5284-
actionName: string,
5285-
actionArgs: Record<string, unknown>,
5286-
): Promise<unknown> => {
5287-
const operation = runtime.operationService.start({
5288-
kind: `ade_action.${actionName}`,
5289-
metadata: {
5290-
actionName,
5291-
callerId: session.identity.callerId,
5292-
callerRole: session.identity.role,
5293-
chatSessionId: session.identity.chatSessionId,
5294-
readOnly: READ_ONLY_TOOLS.has(actionName),
5295-
},
5296-
});
5297-
try {
5298-
const result = await callAction(actionName, actionArgs);
5299-
const failureMessage = actionResultFailureMessage(result);
5300-
if (failureMessage) {
5301-
finishActionAudit(actionName, operation.operationId, "failed", {
5302-
resultStatus: "error",
5303-
error: failureMessage,
5304-
});
5305-
return result;
5306-
}
5307-
finishActionAudit(actionName, operation.operationId, "succeeded", { resultStatus: "success" });
5308-
return result;
5309-
} catch (error) {
5310-
const message = error instanceof Error ? error.message : String(error);
5311-
finishActionAudit(actionName, operation.operationId, "failed", {
5312-
resultStatus: "error",
5313-
error: message,
5314-
});
5315-
throw error;
5316-
}
5317-
};
5318-
53195248
const handler = (async (request: JsonRpcRequest): Promise<unknown | null> => {
53205249
const method = typeof request.method === "string" ? request.method : "";
53215250
const params = safeObject(request.params);
@@ -5511,7 +5440,7 @@ export function createAdeRpcRequestHandler(args: {
55115440
const actionName = assertNonEmptyString(params.name, "name");
55125441
const actionArgs = safeObject(params.arguments);
55135442
try {
5514-
return await callActionWithAudit(actionName, actionArgs);
5443+
return await callAction(actionName, actionArgs);
55155444
} catch (error) {
55165445
const message = error instanceof Error ? error.message : String(error);
55175446
return {

0 commit comments

Comments
 (0)