feat(cron): add bash-kind payload for zero-LLM cron execution#69455
feat(cron): add bash-kind payload for zero-LLM cron execution#69455samgibson-bot wants to merge 1 commit into
Conversation
openclaw-barnacle
Bot
added
app: web-ui
Greptile SummaryThis PR adds a Two blocking issues were found: Confidence Score: 3/5Not safe to merge — two P1 defects (broken cron.update for bash payloads, incorrect delivery-status tracking for webhook mode) need to be fixed first. The two P1 findings are present-tense defects on the primary changed paths:
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Otherwise, hand stdout to the same delivery plumbing used by agentTurn. | ||
| if (result.status === "ok" && result.outputText) { | ||
| try { | ||
| const target = await resolveDeliveryTarget( | ||
| params.cfg, | ||
| undefined, | ||
| { | ||
| channel: job.delivery?.channel, | ||
| to: job.delivery?.to, | ||
| accountId: job.delivery?.accountId, | ||
| }, | ||
| ); | ||
| if (target.ok && job.delivery && job.delivery.mode === "announce") { | ||
| await deliverOutboundPayloads({ | ||
| cfg: params.cfg, | ||
| channel: target.channel, | ||
| to: target.to, | ||
| accountId: target.accountId, | ||
| threadId: target.threadId, | ||
| payloads: [{ text: result.outputText }], | ||
| deps: createOutboundSendDeps(params.deps), | ||
| }); | ||
| return { ...result, delivered: true, deliveryAttempted: true }; | ||
| } | ||
| } catch (err) { | ||
| return { | ||
| ...result, | ||
| status: "error", | ||
| error: `bash delivery failed: ${String(err)}`, | ||
| }; | ||
| } | ||
| } | ||
| return { ...result, delivered: false, deliveryAttempted: false }; |
There was a problem hiding this comment.
delivered: false for bash-kind
The runBashJob handler only checks mode === "announce" (line 363). When delivery.mode === "webhook", the block is skipped entirely and the function returns { ...result, delivered: false, deliveryAttempted: false }. The onEvent handler does fire a webhook via postCronWebhook (using evt.summary), but because delivered is already set to false here, job.state.lastDeliveryStatus will persist as "not-delivered" even when the webhook POST actually succeeded. This corrupts the delivery-status tracking that failure alerting and run-log reporting depend on.
The fix is to also handle mode === "webhook" inside runBashJob — or, if the intent is to rely on onEvent for webhooks, set deliveryAttempted: true and delivered: true when mode === "webhook" so the stored state reflects what actually happens.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/server-cron.ts
Line: 351-383
Comment:
**Webhook mode silently returns `delivered: false` for bash-kind**
The `runBashJob` handler only checks `mode === "announce"` (line 363). When `delivery.mode === "webhook"`, the block is skipped entirely and the function returns `{ ...result, delivered: false, deliveryAttempted: false }`. The `onEvent` handler does fire a webhook via `postCronWebhook` (using `evt.summary`), but because `delivered` is already set to `false` here, `job.state.lastDeliveryStatus` will persist as `"not-delivered"` even when the webhook POST actually succeeded. This corrupts the delivery-status tracking that failure alerting and run-log reporting depend on.
The fix is to also handle `mode === "webhook"` inside `runBashJob` — or, if the intent is to rely on `onEvent` for webhooks, set `deliveryAttempted: true` and `delivered: true` when `mode === "webhook"` so the stored state reflects what actually happens.
How can I resolve this? If you propose a fix, please make it concise.| const target = await resolveDeliveryTarget( | ||
| params.cfg, | ||
| undefined, | ||
| { | ||
| channel: job.delivery?.channel, | ||
| to: job.delivery?.to, | ||
| accountId: job.delivery?.accountId, | ||
| }, | ||
| ); |
There was a problem hiding this comment.
agentId is undefined when resolving bash delivery target
resolveDeliveryTarget is called with undefined as the second argument (the agentId position). The analogous runIsolatedAgentJob path calls resolveCronAgent(job.agentId) first and passes the resolved id. If delivery-target resolution performs any agent-scoped lookup (e.g., for per-agent channel plugins), it will silently fall back to the default agent, which may differ from job.agentId. Consider mirroring the runIsolatedAgentJob pattern:
| const target = await resolveDeliveryTarget( | |
| params.cfg, | |
| undefined, | |
| { | |
| channel: job.delivery?.channel, | |
| to: job.delivery?.to, | |
| accountId: job.delivery?.accountId, | |
| }, | |
| ); | |
| const { agentId, cfg: runtimeConfig } = resolveCronAgent(job.agentId); | |
| const target = await resolveDeliveryTarget( | |
| runtimeConfig, | |
| agentId, | |
| { | |
| channel: job.delivery?.channel, | |
| to: job.delivery?.to, | |
| accountId: job.delivery?.accountId, | |
| }, | |
| ); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/server-cron.ts
Line: 354-362
Comment:
**`agentId` is `undefined` when resolving bash delivery target**
`resolveDeliveryTarget` is called with `undefined` as the second argument (the `agentId` position). The analogous `runIsolatedAgentJob` path calls `resolveCronAgent(job.agentId)` first and passes the resolved id. If delivery-target resolution performs any agent-scoped lookup (e.g., for per-agent channel plugins), it will silently fall back to the default agent, which may differ from `job.agentId`. Consider mirroring the `runIsolatedAgentJob` pattern:
```suggestion
const { agentId, cfg: runtimeConfig } = resolveCronAgent(job.agentId);
const target = await resolveDeliveryTarget(
runtimeConfig,
agentId,
{
channel: job.delivery?.channel,
to: job.delivery?.to,
accountId: job.delivery?.accountId,
},
);
```
How can I resolve this? If you propose a fix, please make it concise.| const onAbort = () => { | ||
| timedOut = true; | ||
| try { | ||
| child.kill("SIGTERM"); | ||
| } catch { | ||
| /* ignore */ | ||
| } | ||
| }; |
There was a problem hiding this comment.
The timeout path (lines 63–78) sends SIGTERM followed by a 5-second deferred SIGKILL. The onAbort handler only sends SIGTERM and has no escalation. A process that catches and ignores SIGTERM will run indefinitely when aborted, potentially leaking the subprocess. Consider mirroring the timeout escalation pattern in onAbort:
| const onAbort = () => { | |
| timedOut = true; | |
| try { | |
| child.kill("SIGTERM"); | |
| } catch { | |
| /* ignore */ | |
| } | |
| }; | |
| const onAbort = () => { | |
| timedOut = true; | |
| try { | |
| child.kill("SIGTERM"); | |
| setTimeout(() => { | |
| try { child.kill("SIGKILL"); } catch { /* already gone */ } | |
| }, 5000).unref(); | |
| } catch { | |
| /* ignore */ | |
| } | |
| }; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cron/bash-run/run.ts
Line: 81-88
Comment:
**Abort path sends SIGTERM but no SIGKILL escalation**
The timeout path (lines 63–78) sends SIGTERM followed by a 5-second deferred SIGKILL. The `onAbort` handler only sends SIGTERM and has no escalation. A process that catches and ignores SIGTERM will run indefinitely when aborted, potentially leaking the subprocess. Consider mirroring the timeout escalation pattern in `onAbort`:
```suggestion
const onAbort = () => {
timedOut = true;
try {
child.kill("SIGTERM");
setTimeout(() => {
try { child.kill("SIGKILL"); } catch { /* already gone */ }
}, 5000).unref();
} catch {
/* ignore */
}
};
```
How can I resolve this? If you propose a fix, please make it concise.Introduce a third CronPayload variant `{ kind: "bash", command, ... }` that
runs the configured shell command directly via /bin/bash -c, captures stdout,
and routes it through the existing delivery plumbing — no agent turn spawned.
## Motivation
Many cron jobs in real-world deployments are pure stdout relays: the payload
tells the isolated agent "run this bash command and print its output verbatim,"
and the agent does nothing but proxy. Every such job pays a full LLM turn per
fire (input tokens for the prompt + system context + tool definitions, output
tokens for the relayed text) to accomplish what `child_process.spawn` could do
for free.
On my own deployment (~20 relay-style crons firing daily to weekly), routing
these to bash-kind would eliminate ~1M LLM tokens per month with no change in
user-visible behavior. The LLM was never doing judgment work on these jobs.
## Behavior
- `payload.kind: "bash"` with required `command: string` field
- Optional `timeoutSeconds` (default 300, max 3600), `cwd`, `env`, `maxOutputBytes`
- Security: runs with gateway's uid/gid, no additional sandboxing (mirrors the
posture of the existing agent `bash` tool — worth discussing in review)
- Delivery: integrates with the same `delivery: { mode, channel, to, accountId }`
contract as `agentTurn`. `announce` sends stdout to the channel; `webhook`
POSTs it; `none` discards.
- Convention: if stdout trims to exactly `NO_REPLY` (or empty), delivery is
suppressed — matches the silent-on-success pattern many existing crons use
inside their payload messages.
- Errors (non-zero exit, timeout, spawn failure) set `lastRunStatus=error` so
the existing backoff + failure-alert pipeline works unchanged.
## Changes
- `src/cron/types.ts` — extend `CronPayload` / `CronPayloadPatch` unions with
`CronBashPayload` / `CronBashPayloadPatch`.
- `src/gateway/protocol/schema/cron.ts` — TypeBox `cronBashPayloadSchema`;
added to `CronPayloadSchema` + `CronPayloadPatchSchema`.
- `src/cron/service/jobs.ts` — `assertSupportedJobSpec` now accepts
`payload.kind=bash` on isolated/current/session session targets.
- `src/cron/bash-run/run.ts` (new) — `runCronBashJob`: spawn, capture, timeout,
size cap, abort-signal integration.
- `src/cron/service/state.ts` — add optional `runBashJob` dep to
`CronServiceDeps`.
- `src/gateway/server-cron.ts` — wire `runBashJob` dep: invokes
`runCronBashJob`, then routes stdout through `deliverOutboundPayloads`
with the same `resolveDeliveryTarget` + `createOutboundSendDeps` pattern as
isolated-agent delivery.
- `src/cron/service/timer.ts` — `executeDetachedCronJob` routes `kind=bash`
to the new dep; `kind=agentTurn` path unchanged.
## What's NOT in this PR (intentionally)
- Unit/e2e tests — would appreciate maintainer guidance on test-fixture style
for subprocess-based cron execution before writing them.
- CLI wiring (`openclaw cron add --bash "command"`) — can be a follow-up PR;
current `cron add` happily accepts a JSON payload with `kind: "bash"` via
the schema.
- Sandboxing beyond process isolation — open question for maintainers: should
bash-kind crons run under a restricted user, seccomp profile, or
container? The existing agent `bash` tool accepts similar trust; this PR
adopts the same model but I'm open to tightening.
- Model-override / thinking / toolsAllow fields — none apply to bash; schema
reflects that via `additionalProperties: false`.
## Compatibility
All changes are additive. `payload.kind=agentTurn` and `=systemEvent` paths
are untouched. Jobs created before this PR remain valid. New field validation
uses `additionalProperties: false` to reject future-incompatible shapes.
## Testing done locally
- Branch builds against the schema structure (visual inspection only; I
haven't run full `pnpm check` locally since the repo needs `pnpm install`
and this is a design-proposal-scale change). I'd like maintainer review on
the overall shape before investing in test fixtures.
- I'd appreciate a tag or comment on which test harness (`server.cron.e2e.test.ts`?
`normalize.test.ts`?) is the right anchor for bash-kind coverage.
## Alternative considered
Per-cron `payload.model` override to route to cheaper models (e.g. Haiku 4.5).
That's already supported by the current code (`run.ts:206`) and cuts cost
meaningfully (15–20x on Haiku vs Sonnet), but still pays a full agent turn
per fire. bash-kind eliminates the agent turn entirely for cases that don't
need one, and the two features compose naturally (keep agentTurn with a
cheap model for jobs that need LLM judgment; use bash-kind for everything
else).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff61a11806
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -153,6 +168,7 @@ export const CronPayloadPatchSchema = Type.Union([ | |||
| message: Type.Optional(NonEmptyString), | |||
| toolsAllow: Type.Union([Type.Array(Type.String()), Type.Null()]), | |||
| }), | |||
| cronBashPayloadSchema({ command: Type.Optional(NonEmptyString) }), | |||
There was a problem hiding this comment.
CronPayloadPatchSchema now accepts kind: "bash", but the service merge path still only has systemEvent/agentTurn branches (mergeCronPayload/buildPayloadFromPatch in src/cron/service/jobs.ts). As a result, updating an existing job to bash (or patching an existing bash payload) falls into the agentTurn validation path and throws cron.update payload.kind="agentTurn" requires message, making the new patch shape unusable.
Useful? React with 👍 / 👎.
| if (target.ok && job.delivery && job.delivery.mode === "announce") { | ||
| await deliverOutboundPayloads({ |
There was a problem hiding this comment.
In the bash execution path, delivery only happens inside if (target.ok && ... mode === "announce"); when target resolution fails, the function falls through and returns an ok result with delivered: false. This silently drops output and bypasses error/backoff/failure-alert handling for common cases like delivery.channel="last" without a resolvable target, unlike the existing agentTurn delivery path which reports delivery-target errors.
Useful? React with 👍 / 👎.
| { | ||
| kind: Type.Literal("bash"), | ||
| command: params.command, | ||
| timeoutSeconds: Type.Optional(Type.Number({ minimum: 1, maximum: 3600 })), |
There was a problem hiding this comment.
The schema advertises bash timeoutSeconds up to 3600 seconds, but the scheduler safety timeout is still computed as 10 minutes for all non-agentTurn payloads (resolveCronJobTimeoutMs in src/cron/service/timeout-policy.ts). That means any bash job configured above 600 seconds is aborted early by the outer timeout, so accepted configuration values do not actually take effect.
Useful? React with 👍 / 👎.
1 participant
feat(cron): add bash-kind payload for zero-LLM cron execution
Introduce a third CronPayload variant
{ kind: "bash", command, ... }thatruns the configured shell command directly via /bin/bash -c, captures stdout,
and routes it through the existing delivery plumbing — no agent turn spawned.
Motivation
Many cron jobs in real-world deployments are pure stdout relays: the payload
tells the isolated agent "run this bash command and print its output verbatim,"
and the agent does nothing but proxy. Every such job pays a full LLM turn per
fire (input tokens for the prompt + system context + tool definitions, output
tokens for the relayed text) to accomplish what
child_process.spawncould dofor free.
On my own deployment (~20 relay-style crons firing daily to weekly), routing
these to bash-kind would eliminate ~1M LLM tokens per month with no change in
user-visible behavior. The LLM was never doing judgment work on these jobs.
Behavior
payload.kind: "bash"with requiredcommand: stringfieldtimeoutSeconds(default 300, max 3600),cwd,env,maxOutputBytesposture of the existing agent
bashtool — worth discussing in review)delivery: { mode, channel, to, accountId }contract as
agentTurn.announcesends stdout to the channel;webhookPOSTs it;
nonediscards.NO_REPLY(or empty), delivery issuppressed — matches the silent-on-success pattern many existing crons use
inside their payload messages.
lastRunStatus=errorsothe existing backoff + failure-alert pipeline works unchanged.
Changes
src/cron/types.ts— extendCronPayload/CronPayloadPatchunions withCronBashPayload/CronBashPayloadPatch.src/gateway/protocol/schema/cron.ts— TypeBoxcronBashPayloadSchema;added to
CronPayloadSchema+CronPayloadPatchSchema.src/cron/service/jobs.ts—assertSupportedJobSpecnow acceptspayload.kind=bashon isolated/current/session session targets.src/cron/bash-run/run.ts(new) —runCronBashJob: spawn, capture, timeout,size cap, abort-signal integration.
src/cron/service/state.ts— add optionalrunBashJobdep toCronServiceDeps.src/gateway/server-cron.ts— wirerunBashJobdep: invokesrunCronBashJob, then routes stdout throughdeliverOutboundPayloadswith the same
resolveDeliveryTarget+createOutboundSendDepspattern asisolated-agent delivery.
src/cron/service/timer.ts—executeDetachedCronJobrouteskind=bashto the new dep;
kind=agentTurnpath unchanged.What's NOT in this PR (intentionally)
for subprocess-based cron execution before writing them.
openclaw cron add --bash "command") — can be a follow-up PR;current
cron addhappily accepts a JSON payload withkind: "bash"viathe schema.
bash-kind crons run under a restricted user, seccomp profile, or
container? The existing agent
bashtool accepts similar trust; this PRadopts the same model but I'm open to tightening.
reflects that via
additionalProperties: false.Compatibility
All changes are additive.
payload.kind=agentTurnand=systemEventpathsare untouched. Jobs created before this PR remain valid. New field validation
uses
additionalProperties: falseto reject future-incompatible shapes.Testing done locally
haven't run full
pnpm checklocally since the repo needspnpm installand this is a design-proposal-scale change). I'd like maintainer review on
the overall shape before investing in test fixtures.
server.cron.e2e.test.ts?normalize.test.ts?) is the right anchor for bash-kind coverage.Alternative considered
Per-cron
payload.modeloverride to route to cheaper models (e.g. Haiku 4.5).That's already supported by the current code (
run.ts:206) and cuts costmeaningfully (15–20x on Haiku vs Sonnet), but still pays a full agent turn
per fire. bash-kind eliminates the agent turn entirely for cases that don't
need one, and the two features compose naturally (keep agentTurn with a
cheap model for jobs that need LLM judgment; use bash-kind for everything
else).