ship: iteration 1 - address review feedback

Author: arul28

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

@@ -3062,6 +3062,45 @@ describe("adeRpcServer", () => {
     expect(finishArgs.metadataPatch?.resultStatus).toBe("success");
   });
 
+  it("does not mask successful action responses when audit finish fails", async () => {
+    const { runtime, operationFinish } = createRuntime();
+    operationFinish.mockImplementationOnce(() => {
+      throw new Error("finish failed");
+    });
+    const handler = createAdeRpcRequestHandler({ runtime, serverVersion: "test" });
+
+    await initialize(handler);
+    const response = await callTool(handler, "list_lanes", {});
+
+    expect(response.isError).toBeUndefined();
+    expect(response.structuredContent.lanes).toHaveLength(2);
+    expect(operationFinish).toHaveBeenCalledTimes(1);
+    expect(runtime.logger.warn).toHaveBeenCalledWith(
+      "ade_rpc.action_audit_finish_failed",
+      expect.objectContaining({ actionName: "list_lanes", status: "succeeded" }),
+    );
+  });
+
+  it("records failed audit metadata for returned action failure payloads", async () => {
+    const { runtime, operationFinish } = createRuntime();
+    runtime.gitService.push.mockResolvedValueOnce({ success: false, error: "push failed" });
+    const handler = createAdeRpcRequestHandler({ runtime, serverVersion: "test" });
+
+    await initialize(handler, { callerId: "agent-1", role: "agent" });
+    const response = await callTool(handler, "run_ade_action", {
+      domain: "git",
+      action: "push",
+      args: { laneId: "lane-1" },
+    });
+
+    expect(response.isError).toBeUndefined();
+    expect(response.structuredContent.result.success).toBe(false);
+    expect(operationFinish).toHaveBeenCalledWith(expect.objectContaining({
+      status: "failed",
+      metadataPatch: expect.objectContaining({ resultStatus: "error", error: "push failed" }),
+    }));
+  });
+
   // ---------- Rate limit tests ----------
 
   afterEach(() => {

apps/ade-cli/src/adeRpcServer.ts

@@ -5245,6 +5245,41 @@ export function createAdeRpcRequestHandler(args: {
     throw new JsonRpcError(JsonRpcErrorCode.methodNotFound, `Unsupported ADE action: ${actionName}`);
   };
 
+  const failedActionPayloadMessage = (value: unknown): string | null => {
+    if (!isRecord(value)) return null;
+    const failed = value.ok === false || value.success === false;
+    if (!failed) return null;
+    const error = value.error;
+    if (typeof error === "string" && error.trim()) return error.trim();
+    if (isRecord(error) && typeof error.message === "string" && error.message.trim()) {
+      return error.message.trim();
+    }
+    return "Action returned a failed result.";
+  };
+
+  const actionResultFailureMessage = (value: unknown): string | null => {
+    if (!isRecord(value)) return null;
+    return failedActionPayloadMessage(value) ?? failedActionPayloadMessage(value.result);
+  };
+
+  const finishActionAudit = (
+    actionName: string,
+    operationId: string,
+    status: "succeeded" | "failed",
+    metadataPatch: Record<string, unknown>,
+  ): void => {
+    try {
+      runtime.operationService.finish({ operationId, status, metadataPatch });
+    } catch (error) {
+      runtime.logger.warn("ade_rpc.action_audit_finish_failed", {
+        actionName,
+        operationId,
+        status,
+        error: error instanceof Error ? error.message : String(error),
+      });
+    }
+  };
+
   const callActionWithAudit = async (
     actionName: string,
     actionArgs: Record<string, unknown>,
@@ -5261,21 +5296,21 @@ export function createAdeRpcRequestHandler(args: {
     });
     try {
       const result = await callAction(actionName, actionArgs);
-      runtime.operationService.finish({
-        operationId: operation.operationId,
-        status: "succeeded",
-        metadataPatch: { resultStatus: "success" },
-      });
+      const failureMessage = actionResultFailureMessage(result);
+      if (failureMessage) {
+        finishActionAudit(actionName, operation.operationId, "failed", {
+          resultStatus: "error",
+          error: failureMessage,
+        });
+        return result;
+      }
+      finishActionAudit(actionName, operation.operationId, "succeeded", { resultStatus: "success" });
       return result;
     } catch (error) {
       const message = error instanceof Error ? error.message : String(error);
-      runtime.operationService.finish({
-        operationId: operation.operationId,
-        status: "failed",
-        metadataPatch: {
-          resultStatus: "error",
-          error: message,
-        },
+      finishActionAudit(actionName, operation.operationId, "failed", {
+        resultStatus: "error",
+        error: message,
       });
       throw error;
     }

apps/desktop/src/main/services/pty/ptyService.ts

@@ -3055,7 +3055,7 @@ export function createPtyService({
         if (chatSessionId) {
           attachedEntry.chatSessionId = chatSessionId;
           terminalChatSessions.set(existingSession.id, chatSessionId);
-          promoteActiveChatTerminal(chatSessionId, existingSession.id, existingSession.toolType);
+          promoteActiveChatTerminal(chatSessionId, existingSession.id, attachedEntry.toolTypeHint);
           if (existingSession.chatSessionId !== chatSessionId) {
             try { sessionService.setChatSessionId(existingSession.id, chatSessionId); } catch {}
           }