mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-24 03:15:46 -06:00
refactor(test): trim agent phase seams (#123714)
* refactor(test): trim agent phase seams * fix(test): preserve direct node cancellation * test(cli): synchronize MCP loopback readiness
This commit is contained in:
committed by
GitHub
parent
f5c46de8ac
commit
0fda32fce9
@@ -154,14 +154,12 @@ export async function invokeNodeSystemRun(params: {
|
||||
...(params.signal ? { signal: params.signal } : {}),
|
||||
}
|
||||
: undefined;
|
||||
const raw = callOptions
|
||||
? await callGatewayTool(
|
||||
"node.invoke",
|
||||
{ timeoutMs: params.invokeWaitMs },
|
||||
params.invoke,
|
||||
callOptions,
|
||||
)
|
||||
: await callGatewayTool("node.invoke", { timeoutMs: params.invokeWaitMs }, params.invoke);
|
||||
const raw = await callGatewayTool(
|
||||
"node.invoke",
|
||||
{ timeoutMs: params.invokeWaitMs },
|
||||
params.invoke,
|
||||
callOptions,
|
||||
);
|
||||
if (typeof asNullableRecord(asNullableRecord(raw)?.payload)?.success !== "boolean") {
|
||||
return {
|
||||
ok: false,
|
||||
|
||||
@@ -74,13 +74,6 @@ describe("invokeNodeSystemRun failure classification", () => {
|
||||
nodeCommandDispatched: false,
|
||||
}),
|
||||
},
|
||||
{
|
||||
name: "disconnect after dispatch",
|
||||
error: gatewayNodeInvokeError({
|
||||
code: "DISCONNECTED",
|
||||
nodeCommandDispatched: true,
|
||||
}),
|
||||
},
|
||||
{
|
||||
name: "missing dispatch provenance",
|
||||
error: gatewayNodeInvokeError({ code: "NOT_CONNECTED" }),
|
||||
@@ -118,8 +111,6 @@ describe("invokeNodeSystemRun failure classification", () => {
|
||||
command: "printf 'one\\ntwo'\necho done",
|
||||
});
|
||||
|
||||
expect(text).toContain("Exec outcome unknown (node=node-1 id=approval-1, outcome-unknown)");
|
||||
expect(text).toContain("The command may have executed. Do not rerun it automatically.");
|
||||
expect(text).toContain("Command:\nprintf 'one\\ntwo'\necho done");
|
||||
});
|
||||
});
|
||||
@@ -159,30 +150,21 @@ describe("direct node run", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ name: "with its original cancellation signal", withSignal: true },
|
||||
{ name: "without a signal argument", withSignal: false },
|
||||
])("forwards the gateway call $name", async ({ withSignal }) => {
|
||||
it("forwards the original cancellation signal to the gateway", async () => {
|
||||
const controller = new AbortController();
|
||||
await invokeNodeSystemRunDirect(
|
||||
createDirectNodeRun(withSignal ? controller.signal : undefined),
|
||||
);
|
||||
await invokeNodeSystemRunDirect(createDirectNodeRun(controller.signal));
|
||||
|
||||
const baseArgs = [
|
||||
expect(callGatewayToolMock).toHaveBeenCalledWith(
|
||||
"node.invoke",
|
||||
{ timeoutMs: 35_000 },
|
||||
expect.objectContaining({ command: "system.run" }),
|
||||
] as const;
|
||||
if (withSignal) {
|
||||
expect(callGatewayToolMock).toHaveBeenCalledWith(...baseArgs, { signal: controller.signal });
|
||||
} else {
|
||||
expect(callGatewayToolMock).toHaveBeenCalledWith(...baseArgs);
|
||||
}
|
||||
{ signal: controller.signal },
|
||||
);
|
||||
});
|
||||
|
||||
it("surfaces capped stderr and the node error when stdout is also present", async () => {
|
||||
it("combines stdout, stderr, and the node error", async () => {
|
||||
const stdout = "small stdout";
|
||||
const stderr = `${"x".repeat(200_000)}\n... (truncated)`;
|
||||
const stderr = "node stderr";
|
||||
const errorText = "node command failed";
|
||||
callGatewayToolMock.mockResolvedValueOnce({
|
||||
payload: {
|
||||
@@ -197,14 +179,8 @@ describe("direct node run", () => {
|
||||
const result = await invokeNodeSystemRunDirect(createDirectNodeRun());
|
||||
const visibleText = result.content[0]?.type === "text" ? result.content[0].text : "";
|
||||
|
||||
expect(visibleText).toContain("... (truncated)");
|
||||
expect(visibleText).toContain(errorText);
|
||||
expect(visibleText).toBe(`${stdout}\n${stderr}\n${errorText}`);
|
||||
expect(result.details).toMatchObject({
|
||||
status: "failed",
|
||||
exitCode: 1,
|
||||
aggregated: visibleText,
|
||||
});
|
||||
expect(result.details).toMatchObject({ aggregated: visibleText });
|
||||
});
|
||||
|
||||
it("never dispatches a direct node run after cancellation", async () => {
|
||||
|
||||
@@ -284,7 +284,6 @@ describe("runEmbeddedAttemptExecutionPhase", () => {
|
||||
const settledInput = mocks.runSettledPhase.mock.calls[0]?.[0];
|
||||
expect(settledInput).toEqual(
|
||||
expect.objectContaining({
|
||||
getRepairedRejectedProviderReplay: expect.any(Function),
|
||||
preparedStreamRuntime: expect.objectContaining({
|
||||
cache: {
|
||||
observabilityEnabled: true,
|
||||
@@ -297,7 +296,6 @@ describe("runEmbeddedAttemptExecutionPhase", () => {
|
||||
}),
|
||||
}),
|
||||
);
|
||||
expect(settledInput.getRepairedRejectedProviderReplay()).toBe(true);
|
||||
|
||||
const guardInput = mocks.installStreamGuards.mock.calls[0]?.[0];
|
||||
expect(guardInput).toEqual(
|
||||
@@ -338,14 +336,11 @@ describe("runEmbeddedAttemptExecutionPhase", () => {
|
||||
expect(mocks.withOwnedSessionTranscriptWrites).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ label: "external cancellation", message: "run cancelled" },
|
||||
{ label: "run timeout", message: "run timed out" },
|
||||
])("does not start a prompt after $label", async ({ message }) => {
|
||||
it("does not start a prompt after external cancellation", async () => {
|
||||
const fixture = createFixture();
|
||||
await runEmbeddedAttemptExecutionPhase(fixture.input);
|
||||
const reason = new Error(message);
|
||||
const abortError = new Error(message, { cause: reason });
|
||||
const reason = new Error("run cancelled");
|
||||
const abortError = new Error("run cancelled", { cause: reason });
|
||||
abortError.name = "AbortError";
|
||||
fixture.input.runAbortController.abort(reason);
|
||||
mocks.abortable.mockImplementationOnce((_signal, _promise) => Promise.reject(abortError));
|
||||
@@ -353,11 +348,9 @@ describe("runEmbeddedAttemptExecutionPhase", () => {
|
||||
|
||||
await expect(
|
||||
settledInput.preparedStreamRuntime.promptActiveSession("must not start"),
|
||||
).rejects.toBe(abortError);
|
||||
).rejects.toThrow("run cancelled");
|
||||
|
||||
expect(fixture.activeSession.prompt).not.toHaveBeenCalled();
|
||||
expect(fixture.trackPromptSettlePromise).not.toHaveBeenCalled();
|
||||
expect(mocks.abortable).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("flushes pending tool results and disposes the session when history preparation fails", async () => {
|
||||
@@ -374,22 +367,5 @@ describe("runEmbeddedAttemptExecutionPhase", () => {
|
||||
timeoutMs: 0,
|
||||
});
|
||||
expect(fixture.activeSession.dispose).toHaveBeenCalledOnce();
|
||||
expect(mocks.createRunAbort).not.toHaveBeenCalled();
|
||||
expect(mocks.prepareStream).not.toHaveBeenCalled();
|
||||
expect(mocks.prepareTimeout).not.toHaveBeenCalled();
|
||||
expect(mocks.runSettledPhase).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not enter settlement when stream preparation fails", async () => {
|
||||
const fixture = createFixture();
|
||||
mocks.prepareStream.mockImplementationOnce(() => {
|
||||
throw new Error("stream setup failed");
|
||||
});
|
||||
|
||||
await expect(runEmbeddedAttemptExecutionPhase(fixture.input)).rejects.toThrow(
|
||||
"stream setup failed",
|
||||
);
|
||||
|
||||
expect(mocks.runSettledPhase).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -58,9 +58,13 @@ async function createWorkspace(): Promise<string> {
|
||||
return dir;
|
||||
}
|
||||
|
||||
async function waitForLog(text: string): Promise<void> {
|
||||
await vi.waitFor(() => {
|
||||
expect(mocks.runtime.log.mock.calls.some(([line]) => String(line).includes(text))).toBe(true);
|
||||
function waitForLog(text: string): Promise<void> {
|
||||
return new Promise((resolve) => {
|
||||
mocks.runtime.log.mockImplementation((line) => {
|
||||
if (String(line).includes(text)) {
|
||||
resolve();
|
||||
}
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
@@ -94,6 +98,7 @@ function mockRedirectFlow(redirectUrl: string): void {
|
||||
describe("mcp login loopback callback", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mocks.runtime.log.mockReset();
|
||||
program = new Command().exitOverride();
|
||||
registerMcpCli(program);
|
||||
mocks.readMcpOAuthCredentialsStatus.mockResolvedValue({
|
||||
@@ -115,8 +120,9 @@ describe("mcp login loopback callback", () => {
|
||||
const redirectUrl = `http://127.0.0.1:${port}/oauth/callback`;
|
||||
mockRedirectFlow(redirectUrl);
|
||||
|
||||
const waitingForBrowser = waitForLog("Waiting for the browser");
|
||||
const login = program.parseAsync(["mcp", "login", "docs"], { from: "user" });
|
||||
await waitForLog("Waiting for the browser");
|
||||
await waitingForBrowser;
|
||||
const printedUrlIndex = mocks.runtime.log.mock.calls.findIndex(([line]) =>
|
||||
String(line).startsWith("https://auth.example.com/authorize"),
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user