From fc3abc139bedbdfb8fba2bc634e95aa3e2bef4f4 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Apr 2026 03:01:47 +0100 Subject: [PATCH] fix(cron): classify denied isolated runs --- CHANGELOG.md | 1 + docs/automation/cron-jobs.md | 1 + docs/cli/cron.md | 5 ++ src/cron/isolated-agent.helpers.test.ts | 71 ++++++++++++++++++++++- src/cron/isolated-agent/helpers.ts | 76 +++++++++++++++++++++++-- 5 files changed, 149 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8061c2a2467..dadbc04ba72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Docs: https://docs.openclaw.ai ### 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. - 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. diff --git a/docs/automation/cron-jobs.md b/docs/automation/cron-jobs.md index c8f072aa42c..fcc57d630ad 100644 --- a/docs/automation/cron-jobs.md +++ b/docs/automation/cron-jobs.md @@ -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. - Isolated cron runs best-effort close tracked browser tabs/processes for their `cron:` 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 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. diff --git a/docs/cli/cron.md b/docs/cli/cron.md index c34939f0a4f..1398d95cb37 100644 --- a/docs/cli/cron.md +++ b/docs/cli/cron.md @@ -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 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. 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 diff --git a/src/cron/isolated-agent.helpers.test.ts b/src/cron/isolated-agent.helpers.test.ts index 74b140bc105..4f8e1fe217c 100644 --- a/src/cron/isolated-agent.helpers.test.ts +++ b/src/cron/isolated-agent.helpers.test.ts @@ -1,5 +1,31 @@ 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", () => { it("uses the last non-empty non-error payload as summary and output", () => { @@ -134,4 +160,47 @@ describe("resolveCronPayloadOutcome", () => { { 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"); + }); }); diff --git a/src/cron/isolated-agent/helpers.ts b/src/cron/isolated-agent/helpers.ts index 1a822215db2..08d8a8b30b5 100644 --- a/src/cron/isolated-agent/helpers.ts +++ b/src/cron/isolated-agent/helpers.ts @@ -21,6 +21,60 @@ export type CronPayloadOutcome = { 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(); + 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) { const clean = (text ?? "").trim(); if (!clean) { @@ -157,7 +211,7 @@ export function resolveCronPayloadOutcome(params: { params.payloads .slice(lastErrorPayloadIndex + 1) .some((payload) => payload?.isError !== true && Boolean(payload?.text?.trim())); - const hasFatalErrorPayload = hasErrorPayload && !hasSuccessfulPayloadAfterLastError; + const hasFatalStructuredErrorPayload = hasErrorPayload && !hasSuccessfulPayloadAfterLastError; const normalizedFinalAssistantVisibleText = normalizeOptionalString( params.finalAssistantVisibleText, ); @@ -169,7 +223,7 @@ export function resolveCronPayloadOutcome(params: { const shouldUseFinalAssistantVisibleText = params.preferFinalAssistantVisibleText === true && normalizedFinalAssistantVisibleText !== undefined && - !hasFatalErrorPayload && + !hasFatalStructuredErrorPayload && !hasStructuredDeliveryPayloads; const summary = shouldUseFinalAssistantVisibleText ? (pickSummaryFromOutput(normalizedFinalAssistantVisibleText) ?? fallbackSummary) @@ -189,6 +243,18 @@ export function resolveCronPayloadOutcome(params: { .toReversed() .find((payload) => payload?.isError === true && Boolean(payload?.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 { summary, outputText, @@ -197,8 +263,10 @@ export function resolveCronPayloadOutcome(params: { deliveryPayloads: resolvedDeliveryPayloads, deliveryPayloadHasStructuredContent, hasFatalErrorPayload, - embeddedRunError: hasFatalErrorPayload + embeddedRunError: hasFatalStructuredErrorPayload ? (lastErrorPayloadText ?? "cron isolated run returned an error payload") - : undefined, + : denialSignal + ? formatCronDenialSignal(denialSignal) + : undefined, }; }