mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 21:10:43 +00:00
fix(cron): classify denied isolated runs
This commit is contained in:
@@ -6,6 +6,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
|
|
||||||
### Fixes
|
### Fixes
|
||||||
|
|
||||||
|
- Cron: classify isolated runs as errors when final output narrates known execution-denial markers such as `SYSTEM_RUN_DENIED`, `INVALID_REQUEST`, or approval-binding refusal phrases, so blocked commands no longer appear green in cron history. Fixes #67172; carries forward #67186. Thanks @oc-gh-dr, @hclsys, and @1yihui.
|
||||||
- macOS Gateway: write launchd services with a state-dir `WorkingDirectory`, use a durable state-dir temp path instead of freezing macOS session `TMPDIR`, create that temp directory before bootstrap, and label abort-shaped launchd exits as `SIGABRT/abort` in status output. Fixes #53679 and #70223; refs #71848. Thanks @dlturock, @stammi922, and @palladius.
|
- macOS Gateway: write launchd services with a state-dir `WorkingDirectory`, use a durable state-dir temp path instead of freezing macOS session `TMPDIR`, create that temp directory before bootstrap, and label abort-shaped launchd exits as `SIGABRT/abort` in status output. Fixes #53679 and #70223; refs #71848. Thanks @dlturock, @stammi922, and @palladius.
|
||||||
- Exec approvals: accept runtime-owned `source: "allow-always"` and `commandText` allowlist metadata in gateway and node approval-set payloads so Control UI round-trips no longer fail with `unexpected property 'source'`. Fixes #60000; carries forward #60064. Thanks @sd1471123, @sharkqwy, and @luoyanglang.
|
- Exec approvals: accept runtime-owned `source: "allow-always"` and `commandText` allowlist metadata in gateway and node approval-set payloads so Control UI round-trips no longer fail with `unexpected property 'source'`. Fixes #60000; carries forward #60064. Thanks @sd1471123, @sharkqwy, and @luoyanglang.
|
||||||
- Exec/node: skip approval-plan preparation for full-trust `host=node` runs so interpreter and script commands no longer fail with `SYSTEM_RUN_DENIED: approval cannot safely bind` when effective policy is `security=full` and `ask=off`. Fixes #48457 and duplicate #69251. Thanks @ajtran303, @jaserNo1, @Blakeshannon, @lesliefag, and @AvIsBeastMC.
|
- Exec/node: skip approval-plan preparation for full-trust `host=node` runs so interpreter and script commands no longer fail with `SYSTEM_RUN_DENIED: approval cannot safely bind` when effective policy is `security=full` and `ask=off`. Fixes #48457 and duplicate #69251. Thanks @ajtran303, @jaserNo1, @Blakeshannon, @lesliefag, and @AvIsBeastMC.
|
||||||
|
|||||||
@@ -47,6 +47,7 @@ Cron is the Gateway's built-in scheduler. It persists jobs, wakes the agent at t
|
|||||||
- One-shot jobs (`--at`) auto-delete after success by default.
|
- One-shot jobs (`--at`) auto-delete after success by default.
|
||||||
- Isolated cron runs best-effort close tracked browser tabs/processes for their `cron:<jobId>` session when the run completes, so detached browser automation does not leave orphaned processes behind.
|
- Isolated cron runs best-effort close tracked browser tabs/processes for their `cron:<jobId>` session when the run completes, so detached browser automation does not leave orphaned processes behind.
|
||||||
- Isolated cron runs also guard against stale acknowledgement replies. If the first result is just an interim status update (`on it`, `pulling everything together`, and similar hints) and no descendant subagent run is still responsible for the final answer, OpenClaw re-prompts once for the actual result before delivery.
|
- Isolated cron runs also guard against stale acknowledgement replies. If the first result is just an interim status update (`on it`, `pulling everything together`, and similar hints) and no descendant subagent run is still responsible for the final answer, OpenClaw re-prompts once for the actual result before delivery.
|
||||||
|
- Isolated cron runs classify known execution-denial markers in the final summary/output as failures, including host markers such as `SYSTEM_RUN_DENIED` and `INVALID_REQUEST`, so a blocked command is not reported as a green run.
|
||||||
|
|
||||||
<a id="maintenance"></a>
|
<a id="maintenance"></a>
|
||||||
|
|
||||||
|
|||||||
@@ -57,6 +57,11 @@ Note: if an isolated cron run returns only the silent token (`NO_REPLY` /
|
|||||||
`no_reply`), cron suppresses direct outbound delivery and the fallback queued
|
`no_reply`), cron suppresses direct outbound delivery and the fallback queued
|
||||||
summary path as well, so nothing is posted back to chat.
|
summary path as well, so nothing is posted back to chat.
|
||||||
|
|
||||||
|
Note: isolated cron runs treat known denial markers in final output, such as
|
||||||
|
`SYSTEM_RUN_DENIED`, `INVALID_REQUEST`, and approval-binding refusal phrases, as
|
||||||
|
errors. `cron list` and run history then surface the matched token in the error
|
||||||
|
reason instead of reporting a blocked command as `ok`.
|
||||||
|
|
||||||
Note: `cron add|edit --model ...` uses that selected allowed model for the job.
|
Note: `cron add|edit --model ...` uses that selected allowed model for the job.
|
||||||
If the model is not allowed, cron warns and falls back to the job's agent/default
|
If the model is not allowed, cron warns and falls back to the job's agent/default
|
||||||
model selection instead. Configured fallback chains still apply, but a plain
|
model selection instead. Configured fallback chains still apply, but a plain
|
||||||
|
|||||||
@@ -1,5 +1,31 @@
|
|||||||
import { describe, expect, it } from "vitest";
|
import { describe, expect, it } from "vitest";
|
||||||
import { resolveCronPayloadOutcome } from "./isolated-agent/helpers.js";
|
import { detectCronDenialToken, resolveCronPayloadOutcome } from "./isolated-agent/helpers.js";
|
||||||
|
|
||||||
|
describe("detectCronDenialToken", () => {
|
||||||
|
it("matches host denial markers case-sensitively", () => {
|
||||||
|
expect(detectCronDenialToken("SYSTEM_RUN_DENIED: approval blocked")).toBe("SYSTEM_RUN_DENIED");
|
||||||
|
expect(detectCronDenialToken("INVALID_REQUEST: denied")).toBe("INVALID_REQUEST");
|
||||||
|
expect(detectCronDenialToken("system_run_denied: approval blocked")).toBeUndefined();
|
||||||
|
expect(detectCronDenialToken("invalid_request: denied")).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("matches model-narrated denial phrases case-insensitively", () => {
|
||||||
|
expect(detectCronDenialToken("Approval Cannot Safely Bind this runtime command")).toBe(
|
||||||
|
"approval cannot safely bind",
|
||||||
|
);
|
||||||
|
expect(detectCronDenialToken("The runtime denied the operation.")).toBe("runtime denied");
|
||||||
|
expect(detectCronDenialToken("I could not run the script.")).toBe("could not run");
|
||||||
|
expect(detectCronDenialToken("The command did not run to completion.")).toBe("did not run");
|
||||||
|
expect(detectCronDenialToken("The request was denied by policy.")).toBe("was denied");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("ignores empty and non-token text", () => {
|
||||||
|
expect(detectCronDenialToken(undefined)).toBeUndefined();
|
||||||
|
expect(
|
||||||
|
detectCronDenialToken("The denied claim was reviewed, then the job succeeded."),
|
||||||
|
).toBeUndefined();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("resolveCronPayloadOutcome", () => {
|
describe("resolveCronPayloadOutcome", () => {
|
||||||
it("uses the last non-empty non-error payload as summary and output", () => {
|
it("uses the last non-empty non-error payload as summary and output", () => {
|
||||||
@@ -134,4 +160,47 @@ describe("resolveCronPayloadOutcome", () => {
|
|||||||
{ text: "Final weather summary" },
|
{ text: "Final weather summary" },
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("promotes narrated denial markers in summary text to fatal errors", () => {
|
||||||
|
const result = resolveCronPayloadOutcome({
|
||||||
|
payloads: [
|
||||||
|
{
|
||||||
|
text: "SYSTEM_RUN_DENIED: approval cannot safely bind this interpreter/runtime command",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.hasFatalErrorPayload).toBe(true);
|
||||||
|
expect(result.embeddedRunError).toBe(
|
||||||
|
'cron classifier: denial token "SYSTEM_RUN_DENIED" detected in summary',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("promotes narrated denial markers from final assistant visible text", () => {
|
||||||
|
const result = resolveCronPayloadOutcome({
|
||||||
|
payloads: [{ text: "Working on it..." }],
|
||||||
|
finalAssistantVisibleText: "I could not run the requested script.",
|
||||||
|
preferFinalAssistantVisibleText: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.hasFatalErrorPayload).toBe(true);
|
||||||
|
expect(result.outputText).toBe("I could not run the requested script.");
|
||||||
|
expect(result.embeddedRunError).toBe(
|
||||||
|
'cron classifier: denial token "could not run" detected in summary',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps structured error payload reasons ahead of denial-token reasons", () => {
|
||||||
|
const result = resolveCronPayloadOutcome({
|
||||||
|
payloads: [
|
||||||
|
{
|
||||||
|
text: "Exec failed before SYSTEM_RUN_DENIED could be retried",
|
||||||
|
isError: true,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.hasFatalErrorPayload).toBe(true);
|
||||||
|
expect(result.embeddedRunError).toBe("Exec failed before SYSTEM_RUN_DENIED could be retried");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -21,6 +21,60 @@ export type CronPayloadOutcome = {
|
|||||||
embeddedRunError?: string;
|
embeddedRunError?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
type CronDenialSignal = {
|
||||||
|
token: string;
|
||||||
|
field: string;
|
||||||
|
};
|
||||||
|
|
||||||
|
const CRON_DENIAL_EXACT_TOKENS = ["SYSTEM_RUN_DENIED", "INVALID_REQUEST"] as const;
|
||||||
|
const CRON_DENIAL_CASE_INSENSITIVE_TOKENS = [
|
||||||
|
"approval cannot safely bind",
|
||||||
|
"runtime denied",
|
||||||
|
"could not run",
|
||||||
|
"did not run",
|
||||||
|
"was denied",
|
||||||
|
] as const;
|
||||||
|
|
||||||
|
export function detectCronDenialToken(text: string | undefined): string | undefined {
|
||||||
|
const normalized = normalizeOptionalString(text);
|
||||||
|
if (!normalized) {
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
for (const token of CRON_DENIAL_EXACT_TOKENS) {
|
||||||
|
if (normalized.includes(token)) {
|
||||||
|
return token;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
const lowerText = normalized.toLowerCase();
|
||||||
|
for (const token of CRON_DENIAL_CASE_INSENSITIVE_TOKENS) {
|
||||||
|
if (lowerText.includes(token)) {
|
||||||
|
return token;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
function resolveCronDenialSignal(
|
||||||
|
fields: Array<{ field: string; text?: string | undefined }>,
|
||||||
|
): CronDenialSignal | undefined {
|
||||||
|
const seen = new Set<string>();
|
||||||
|
for (const { field, text } of fields) {
|
||||||
|
if (seen.has(field)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
seen.add(field);
|
||||||
|
const token = detectCronDenialToken(text);
|
||||||
|
if (token) {
|
||||||
|
return { token, field };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
function formatCronDenialSignal(signal: CronDenialSignal): string {
|
||||||
|
return `cron classifier: denial token "${signal.token}" detected in ${signal.field}`;
|
||||||
|
}
|
||||||
|
|
||||||
export function pickSummaryFromOutput(text: string | undefined) {
|
export function pickSummaryFromOutput(text: string | undefined) {
|
||||||
const clean = (text ?? "").trim();
|
const clean = (text ?? "").trim();
|
||||||
if (!clean) {
|
if (!clean) {
|
||||||
@@ -157,7 +211,7 @@ export function resolveCronPayloadOutcome(params: {
|
|||||||
params.payloads
|
params.payloads
|
||||||
.slice(lastErrorPayloadIndex + 1)
|
.slice(lastErrorPayloadIndex + 1)
|
||||||
.some((payload) => payload?.isError !== true && Boolean(payload?.text?.trim()));
|
.some((payload) => payload?.isError !== true && Boolean(payload?.text?.trim()));
|
||||||
const hasFatalErrorPayload = hasErrorPayload && !hasSuccessfulPayloadAfterLastError;
|
const hasFatalStructuredErrorPayload = hasErrorPayload && !hasSuccessfulPayloadAfterLastError;
|
||||||
const normalizedFinalAssistantVisibleText = normalizeOptionalString(
|
const normalizedFinalAssistantVisibleText = normalizeOptionalString(
|
||||||
params.finalAssistantVisibleText,
|
params.finalAssistantVisibleText,
|
||||||
);
|
);
|
||||||
@@ -169,7 +223,7 @@ export function resolveCronPayloadOutcome(params: {
|
|||||||
const shouldUseFinalAssistantVisibleText =
|
const shouldUseFinalAssistantVisibleText =
|
||||||
params.preferFinalAssistantVisibleText === true &&
|
params.preferFinalAssistantVisibleText === true &&
|
||||||
normalizedFinalAssistantVisibleText !== undefined &&
|
normalizedFinalAssistantVisibleText !== undefined &&
|
||||||
!hasFatalErrorPayload &&
|
!hasFatalStructuredErrorPayload &&
|
||||||
!hasStructuredDeliveryPayloads;
|
!hasStructuredDeliveryPayloads;
|
||||||
const summary = shouldUseFinalAssistantVisibleText
|
const summary = shouldUseFinalAssistantVisibleText
|
||||||
? (pickSummaryFromOutput(normalizedFinalAssistantVisibleText) ?? fallbackSummary)
|
? (pickSummaryFromOutput(normalizedFinalAssistantVisibleText) ?? fallbackSummary)
|
||||||
@@ -189,6 +243,18 @@ export function resolveCronPayloadOutcome(params: {
|
|||||||
.toReversed()
|
.toReversed()
|
||||||
.find((payload) => payload?.isError === true && Boolean(payload?.text?.trim()))
|
.find((payload) => payload?.isError === true && Boolean(payload?.text?.trim()))
|
||||||
?.text?.trim();
|
?.text?.trim();
|
||||||
|
const denialSignal = resolveCronDenialSignal([
|
||||||
|
{ field: "summary", text: summary },
|
||||||
|
{ field: "outputText", text: outputText },
|
||||||
|
{ field: "synthesizedText", text: synthesizedText },
|
||||||
|
{ field: "fallbackSummary", text: fallbackSummary },
|
||||||
|
{ field: "fallbackOutputText", text: fallbackOutputText },
|
||||||
|
...params.payloads.map((payload, index) => ({
|
||||||
|
field: `payloads[${index}].text`,
|
||||||
|
text: payload?.text,
|
||||||
|
})),
|
||||||
|
]);
|
||||||
|
const hasFatalErrorPayload = hasFatalStructuredErrorPayload || denialSignal !== undefined;
|
||||||
return {
|
return {
|
||||||
summary,
|
summary,
|
||||||
outputText,
|
outputText,
|
||||||
@@ -197,8 +263,10 @@ export function resolveCronPayloadOutcome(params: {
|
|||||||
deliveryPayloads: resolvedDeliveryPayloads,
|
deliveryPayloads: resolvedDeliveryPayloads,
|
||||||
deliveryPayloadHasStructuredContent,
|
deliveryPayloadHasStructuredContent,
|
||||||
hasFatalErrorPayload,
|
hasFatalErrorPayload,
|
||||||
embeddedRunError: hasFatalErrorPayload
|
embeddedRunError: hasFatalStructuredErrorPayload
|
||||||
? (lastErrorPayloadText ?? "cron isolated run returned an error payload")
|
? (lastErrorPayloadText ?? "cron isolated run returned an error payload")
|
||||||
: undefined,
|
: denialSignal
|
||||||
|
? formatCronDenialSignal(denialSignal)
|
||||||
|
: undefined,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user