mirror of
https://github.com/openclaw/lobster.git
synced 2026-08-14 00:48:09 +00:00
* fix(llm): honor step timeouts and cancellation during model calls llm.invoke never read ctx.signal, and both adapter helpers called fetch without one, so nothing could end a request once it was in flight. A workflow step with timeout_ms kept waiting for the adapter and, when the model eventually answered, reported the step as completed well past its budget. Shell steps already honor the same signal, so the two step kinds disagreed about what timeout_ms means. Thread ctx.signal into both adapter fetch calls, check it before each attempt so an already-cancelled run does not pay for another call, and rethrow abort errors unwrapped: the workflow runner recognizes timeouts and external cancellation by the error's identity, so wrapping them in "request failed" would misreport the outcome even once the request stops. * fix(llm): stop waiting for adapters that ignore the cancellation signal Threading the signal into fetch only covers the HTTP adapters. An injected ctx.llmAdapters adapter is awaited directly, so one that never observes ctx.signal still holds a timed-out step open for as long as it likes, which leaves SDK and tool-runtime users on the weaker timeout contract the rest of this change removes. Await the adapter call against the signal instead of the adapter alone. The adapter's own work cannot be killed from here, but the step stops waiting on it and the abort reason reaches the workflow unchanged. Cooperative adapters are unaffected: they still receive ctx and can end their own work. * fix(llm): do not finish a cancelled run from cached results Run-state and file-cache hits return before any adapter call, so a run that was already cancelled still completed successfully from cached data. That contradicts the rest of this change: the same invocation reports cancellation when it has to reach a model and success when it does not, purely on cache contents. Check the signal once at entry, before either lookup. The retry-loop check stays, since cancellation can also arrive between validation attempts. * docs(readme): state the cancellation contract for injected LLM adapters Racing an adapter promise against the step signal restores workflow liveness, but it cannot cancel work the adapter has already started: an injected adapter that ignores `ctx.signal` keeps its model request running after Lobster stops awaiting it, and a configured retry can then overlap it. That is a contract for the host to meet, so the host-facing docs have to say it rather than leaving it to be discovered from a duplicated charge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Bu6MvbjKGtfRiNesARMyZ * fix(llm): re-check cancellation after each awaited reuse boundary The entry check only caught a run that was already cancelled when the command started. Draining pipeline input waits on the upstream step, and the run-state and cache lookups are file I/O, so a step timeout could fire during any of them and the reuse branch would still return its stored answer as a successful result. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011gD5sJTbh2jn1uvgNCmkLq * fix(llm): do not report a step that finished after its deadline The cancellation checks stopped at the adapter call, but both return paths then await run-state and cache writes. A deadline crossed during those writes was never observed, so the step returned success and the workflow cleared its timer -- a model step could still complete late. The check goes after both writes rather than between them: the answer has already been paid for, and leaving it stored lets the retry replay it instead of calling the model a second time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011gD5sJTbh2jn1uvgNCmkLq * test(llm): let the persistence tests run where the state fix has not landed The two cancellation-during-write tests let the command create the cache namespace directory itself, which is where #125 throws on Windows: the recursive mkdir reports an extended-length path and the chain sync then opens `<dir>\C:`. Both tests failed there before reaching an assertion, so this branch added two Windows failures over main for a reason that is not its own. Creating the directory up front is what the seeded-cache tests in this file already do. ensureDirectory only syncs a chain it created, so an existing directory never reaches the broken path, and the tests still fail against the code they cover -- now on the missing rejection rather than on ENOENT. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EwU1bvb9Pt3cR662P52CTp * fix(llm): let a host's own abort reason still read as cancellation The workflow runner classifies cancellation by name alone: only `AbortError` or `ABORT_ERR` bypasses the step's retry and `on_error` policy. Every abort path here rejected with `signal.reason` untouched, so a host that calls `controller.abort(new Error("stop"))` handed the runner an ordinary-looking error. A cancelled `llm.invoke` step under `on_error: continue` was recorded as a step failure and the run carried on -- returning `status: "ok"` for a run the host had cancelled. The gap is narrow, which is why it survived earlier review. An HTTP adapter is saved by fetch rejecting with its own `AbortError`, and the runner's own step timer is not affected. It opens only on the injected `ctx.llmAdapters` path, where this command's rejection is the only thing the runner ever sees. Normalize at the boundary instead of guessing upstream: a reason that already reads as an abort passes through untouched, anything else keeps its message and moves to `cause` under an abort-identified error. The host's reason is never discarded, and callers that already match on `AbortError` keep matching. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WHhk7xRwXzYcSXvw2kp5F5 * fix(llm): watch an adapter that rejects after aborting the run `abortable` guards against an adapter that ignores `ctx.signal`, but its already-aborted branch rejected the wrapper and returned before anything was attached to the adapter's own promise. An adapter that cancels the run from inside its own `invoke` -- an SDK client tearing itself down on a fatal error -- and then rejects leaves that rejection unobserved. The step still fails correctly, with the `AbortError` the runner needs. The damage lands afterwards: under Node's default unhandled-rejection handling the process exits on an error nothing is waiting for, seconds after the run was cancelled cleanly. Attach settlement handling first in both branches, so the promise is watched before an abort can win the race. Removing a listener that was never added is harmless, which keeps the two paths one shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hFZxjivm9JTTvmRjQ4A8S --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
300 lines
8.4 KiB
TypeScript
300 lines
8.4 KiB
TypeScript
import test from "node:test";
|
|
import assert from "node:assert/strict";
|
|
import { promises as fsp } from "node:fs";
|
|
import http from "node:http";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { PassThrough } from "node:stream";
|
|
|
|
import { loadWorkflowFile, runWorkflowFile } from "../src/workflows/file.js";
|
|
import { createDefaultRegistry } from "../src/commands/registry.js";
|
|
|
|
async function runWorkflow(
|
|
workflow: unknown,
|
|
opts?: {
|
|
signal?: AbortSignal;
|
|
dryRun?: boolean;
|
|
env?: Record<string, string>;
|
|
llmAdapters?: Record<string, unknown>;
|
|
},
|
|
) {
|
|
const tmpDir = await fsp.mkdtemp(path.join(os.tmpdir(), "lobster-step-timeout-"));
|
|
const stateDir = path.join(tmpDir, "state");
|
|
const filePath = path.join(tmpDir, "workflow.lobster");
|
|
await fsp.writeFile(filePath, JSON.stringify(workflow, null, 2), "utf8");
|
|
|
|
const stderr = new PassThrough();
|
|
const chunks: string[] = [];
|
|
stderr.on("data", (chunk: Buffer | string) => chunks.push(String(chunk)));
|
|
|
|
const result = await runWorkflowFile({
|
|
filePath,
|
|
ctx: {
|
|
stdin: process.stdin,
|
|
stdout: process.stdout,
|
|
stderr,
|
|
env: { ...process.env, LOBSTER_STATE_DIR: stateDir, ...opts?.env },
|
|
mode: "tool",
|
|
signal: opts?.signal,
|
|
dryRun: opts?.dryRun,
|
|
llmAdapters: opts?.llmAdapters,
|
|
registry: createDefaultRegistry(),
|
|
},
|
|
});
|
|
|
|
return { result, stderrOutput: chunks.join("") };
|
|
}
|
|
|
|
async function writeWorkflow(workflow: unknown) {
|
|
const tmpDir = await fsp.mkdtemp(path.join(os.tmpdir(), "lobster-step-timeout-load-"));
|
|
const filePath = path.join(tmpDir, "workflow.lobster");
|
|
await fsp.writeFile(filePath, JSON.stringify(workflow, null, 2), "utf8");
|
|
return filePath;
|
|
}
|
|
|
|
test("timeout_ms validation rejects non-numeric values", async () => {
|
|
const filePath = await writeWorkflow({
|
|
steps: [{ id: "x", command: "echo hi", timeout_ms: "fast" }],
|
|
});
|
|
await assert.rejects(
|
|
() => loadWorkflowFile(filePath),
|
|
/timeout_ms must be a positive integer between 1 and 2147483647/,
|
|
);
|
|
});
|
|
|
|
test("timeout_ms validation rejects 0", async () => {
|
|
const filePath = await writeWorkflow({
|
|
steps: [{ id: "x", command: "echo hi", timeout_ms: 0 }],
|
|
});
|
|
await assert.rejects(
|
|
() => loadWorkflowFile(filePath),
|
|
/timeout_ms must be a positive integer between 1 and 2147483647/,
|
|
);
|
|
});
|
|
|
|
test("timeout_ms validation rejects non-integer values", async () => {
|
|
const filePath = await writeWorkflow({
|
|
steps: [{ id: "x", command: "echo hi", timeout_ms: 1.5 }],
|
|
});
|
|
await assert.rejects(
|
|
() => loadWorkflowFile(filePath),
|
|
/timeout_ms must be a positive integer between 1 and 2147483647/,
|
|
);
|
|
});
|
|
|
|
test("timeout_ms validation rejects values above Node timer max", async () => {
|
|
const filePath = await writeWorkflow({
|
|
steps: [{ id: "x", command: "echo hi", timeout_ms: 2_147_483_648 }],
|
|
});
|
|
await assert.rejects(
|
|
() => loadWorkflowFile(filePath),
|
|
/timeout_ms must be a positive integer between 1 and 2147483647/,
|
|
);
|
|
});
|
|
|
|
test("on_error validation rejects unsupported values", async () => {
|
|
const filePath = await writeWorkflow({
|
|
steps: [{ id: "x", command: "echo hi", on_error: "retry" }],
|
|
});
|
|
await assert.rejects(
|
|
() => loadWorkflowFile(filePath),
|
|
/on_error must be "stop", "continue", or "skip_rest"/,
|
|
);
|
|
});
|
|
|
|
test("timed-out step fails by default (on_error: stop)", async () => {
|
|
await assert.rejects(
|
|
() =>
|
|
runWorkflow({
|
|
steps: [{ id: "slow", command: 'node -e "setTimeout(() => {}, 5000)"', timeout_ms: 100 }],
|
|
}),
|
|
/timed out after 100ms/,
|
|
);
|
|
});
|
|
|
|
test("timed-out step with on_error: continue records error and continues", async () => {
|
|
const { result } = await runWorkflow({
|
|
steps: [
|
|
{
|
|
id: "slow",
|
|
command: 'node -e "setTimeout(() => {}, 5000)"',
|
|
timeout_ms: 100,
|
|
on_error: "continue",
|
|
},
|
|
{ id: "after", command: 'node -e "process.stdout.write(JSON.stringify({ok:true}))"' },
|
|
],
|
|
});
|
|
assert.equal(result.status, "ok");
|
|
assert.deepEqual(result.output, [{ ok: true }]);
|
|
});
|
|
|
|
test("timed-out step with on_error: skip_rest stops remaining steps", async () => {
|
|
const { result } = await runWorkflow({
|
|
steps: [
|
|
{ id: "start", command: 'node -e "process.stdout.write(JSON.stringify({kept:true}))"' },
|
|
{
|
|
id: "slow",
|
|
command: 'node -e "setTimeout(() => {}, 5000)"',
|
|
timeout_ms: 100,
|
|
on_error: "skip_rest",
|
|
},
|
|
{ id: "after", command: 'node -e "process.stdout.write(JSON.stringify({shouldRun:false}))"' },
|
|
],
|
|
});
|
|
assert.equal(result.status, "ok");
|
|
assert.deepEqual(result.output, [{ kept: true }]);
|
|
});
|
|
|
|
test("step error marker is available to conditions after on_error: continue", async () => {
|
|
const { result } = await runWorkflow({
|
|
steps: [
|
|
{
|
|
id: "slow",
|
|
command: 'node -e "setTimeout(() => {}, 5000)"',
|
|
timeout_ms: 100,
|
|
on_error: "continue",
|
|
},
|
|
{
|
|
id: "check",
|
|
command:
|
|
'node -e "process.stdout.write(JSON.stringify({timedOut: process.env.TIMED_OUT}))"',
|
|
env: {
|
|
TIMED_OUT: "$slow.error",
|
|
},
|
|
when: "$slow.error == true",
|
|
},
|
|
],
|
|
});
|
|
assert.equal(result.status, "ok");
|
|
assert.deepEqual(result.output, [{ timedOut: "true" }]);
|
|
});
|
|
|
|
test("external abort still propagates when timeout is configured", async () => {
|
|
const controller = new AbortController();
|
|
controller.abort();
|
|
|
|
await assert.rejects(
|
|
() =>
|
|
runWorkflow(
|
|
{
|
|
steps: [
|
|
{
|
|
id: "slow",
|
|
command: 'node -e "setTimeout(() => {}, 5000)"',
|
|
timeout_ms: 5000,
|
|
on_error: "continue",
|
|
},
|
|
],
|
|
},
|
|
{ signal: controller.signal },
|
|
),
|
|
(err: any) => err?.name === "AbortError" || err?.code === "ABORT_ERR",
|
|
);
|
|
});
|
|
|
|
test("timed-out llm.invoke step stops waiting for the adapter", { timeout: 20_000 }, async () => {
|
|
const sockets = new Set<import("node:net").Socket>();
|
|
let markClosed = () => {};
|
|
const requestClosed = new Promise<void>((resolve) => (markClosed = resolve));
|
|
|
|
// An adapter that accepts the request and never answers.
|
|
const server = http.createServer((req, res) => {
|
|
req.resume();
|
|
req.on("end", () => res.on("close", () => markClosed()));
|
|
});
|
|
server.on("connection", (socket) => {
|
|
sockets.add(socket);
|
|
socket.on("close", () => sockets.delete(socket));
|
|
});
|
|
|
|
await new Promise<void>((resolve) => server.listen(0, resolve));
|
|
const addr = server.address();
|
|
const port = typeof addr === "object" && addr ? addr.port : 0;
|
|
|
|
try {
|
|
const started = Date.now();
|
|
await assert.rejects(
|
|
() =>
|
|
runWorkflow(
|
|
{
|
|
steps: [
|
|
{
|
|
id: "ask",
|
|
pipeline: 'llm.invoke --prompt "Summarize" --disable-cache',
|
|
timeout_ms: 300,
|
|
},
|
|
],
|
|
},
|
|
{ env: { LOBSTER_LLM_ADAPTER_URL: `http://127.0.0.1:${port}/invoke` } },
|
|
),
|
|
/timed out after 300ms/,
|
|
);
|
|
assert.ok(Date.now() - started < 10_000, "the step must not wait for the adapter");
|
|
await requestClosed;
|
|
} finally {
|
|
for (const socket of sockets) socket.destroy();
|
|
await new Promise<void>((resolve) => server.close(() => resolve()));
|
|
}
|
|
});
|
|
|
|
test("dry-run renders timeout and on_error details", async () => {
|
|
const { stderrOutput } = await runWorkflow(
|
|
{
|
|
steps: [
|
|
{
|
|
id: "fetch",
|
|
command: "curl https://example.com",
|
|
timeout_ms: 5000,
|
|
on_error: "continue",
|
|
},
|
|
],
|
|
},
|
|
{ dryRun: true },
|
|
);
|
|
assert.match(stderrOutput, /timeout: 5000ms/);
|
|
assert.match(stderrOutput, /on_error: continue/);
|
|
});
|
|
|
|
test(
|
|
"external abort with a custom reason still stops the workflow",
|
|
{ timeout: 20_000 },
|
|
async () => {
|
|
let invoked = () => {};
|
|
const adapterInvoked = new Promise<void>((resolve) => (invoked = resolve));
|
|
// Ignores ctx.signal, so what the runner classifies is llm.invoke's own
|
|
// abort rejection rather than a cancelled socket.
|
|
const stubborn = {
|
|
invoke() {
|
|
invoked();
|
|
return new Promise<never>(() => {});
|
|
},
|
|
};
|
|
|
|
const controller = new AbortController();
|
|
// A host may abort with any reason. A plain Error must still read as
|
|
// cancellation, not as an ordinary step failure.
|
|
void adapterInvoked.then(() => controller.abort(new Error("cancelled by host")));
|
|
|
|
await assert.rejects(
|
|
() =>
|
|
runWorkflow(
|
|
{
|
|
steps: [
|
|
{
|
|
id: "ask",
|
|
// on_error: continue is what makes a misclassified abort
|
|
// visible -- the run would otherwise report success.
|
|
pipeline: 'llm.invoke --provider stubborn --prompt "Summarize" --disable-cache',
|
|
on_error: "continue",
|
|
},
|
|
],
|
|
},
|
|
{ signal: controller.signal, llmAdapters: { stubborn } },
|
|
),
|
|
(err: any) =>
|
|
(err?.name === "AbortError" || err?.code === "ABORT_ERR") &&
|
|
/cancelled by host/.test(String(err?.message)),
|
|
);
|
|
},
|
|
);
|