fix(cron): consolidate announce delivery, fire-and-forget trigger, and minimal prompt mode by tyler6204 · Pull Request #40204 · openclaw/openclaw

tyler6204 · 2026-03-08T19:40:02Z

Summary

keep subagent announces spawned from isolated cron runs internal so cron announce delivery stays the single external delivery path
consolidate descendant fallback output into the cron announce payload when the parent reply is still an interim placeholder
add a detached manual run path for gateway cron.run so the API returns immediately after enqueueing background work
use MINIMAL prompt mode for cron isolated agent turn sessions (they were wastefully loading the full chat prompt with SOUL.md, TOOLS.md, etc. when they only need tool access and a focused task instruction)

Testing

pnpm exec vitest run src/agents/pi-embedded-runner/run/attempt.test.ts src/agents/subagent-announce.timeout.test.ts src/cron/isolated-agent/delivery-dispatch.double-announce.test.ts src/gateway/server.cron.test.ts
pnpm test
- unrelated existing failures reproduced locally in src/media/store.redirect.test.ts and src/process/supervisor/adapters/child.test.ts

Closes #40178
Closes #40192

aisle-research-bot · 2026-03-08T19:40:08Z

🔒 Aisle Security Analysis

We found 2 potential security issue(s) in this PR:

#	Severity	Title
1	🟠 High	Unbounded in-memory cron command queue enables resource exhaustion via repeated cron.run enqueues
2	🟡 Medium	Spoofable cron session keys treated as internal suppress subagent announcement delivery

1. 🟠 Unbounded in-memory cron command queue enables resource exhaustion via repeated cron.run enqueues

Property	Value
Severity	High
CWE	CWE-400
Location	`src/cron/service/ops.ts:527-562`

Description

The new cron.run behavior enqueues a background task (context.cron.enqueueRun) rather than executing immediately. The enqueue path has no queue length cap, backpressure, or per-caller throttling, which allows an authorized caller to rapidly create an arbitrarily large in-process backlog.

Key observations:

enqueueRun() always schedules a new queued task when the job is runnable, and returns success immediately.
The underlying queue implementation (enqueueCommandInLane) stores each task closure and Promise capability in memory (state.queue.push(...)) with no maximum size, so repeated requests can grow memory usage until OOM.
Because inspectManualRunDisposition() runs before the queued task sets runningAtMs, a burst of requests can enqueue many mode:"force" runs before the first queued run marks the job as running. With forced mode, each queued task will then execute sequentially, sustaining CPU/network load.
Log amplification: for each queued item that waits >5s, onWait emits a warning, enabling log flooding proportional to the queue size.

Vulnerable code (enqueue without bounds):

// src/cron/service/ops.ts
void enqueueCommandInLane(
  CommandLane.Cron,
  async () => run(state, id, mode),
  { warnAfterMs: 5_000, onWait: (...) => log.warn(...) },
);
return { ok: true, enqueued: true, runId };

Underlying queue has no size limit:

// src/process/command-queue.ts
state.queue.push({ task, resolve, reject, ... });

Impact:

Remote denial of service of the gateway process via memory exhaustion and/or sustained background job execution, by spamming cron.run (admin-scoped method).
Operational impact from excessive warning/error logs when the queue grows.

Recommendation

Implement backpressure and abuse controls for queued manual runs:

Cap the queue depth for CommandLane.Cron (and/or globally) and fail fast when exceeded.
Rate limit cron.run per authenticated client/device and/or per job ID.
Coalesce/dedupe queued manual runs per job (e.g., allow at most one pending manual run per job unless explicitly requested).
Throttle onWait logging (sample logs or log once per job/runId).

Example (queue cap in enqueueCommandInLane):

// src/process/command-queue.ts
const MAX_QUEUE_DEPTH_BY_LANE: Record<string, number> = {
  [CommandLane.Cron]: 100,
  [CommandLane.Main]: 500,
};

export function enqueueCommandInLane<T>(lane: string, task: () => Promise<T>, opts?: ...) {
  ...
  const maxDepth = MAX_QUEUE_DEPTH_BY_LANE[cleaned] ?? 500;
  const depth = state.queue.length + state.activeTaskIds.size;
  if (depth >= maxDepth) {
    return Promise.reject(new Error(`lane queue full: lane=${cleaned} depth=${depth}`));
  }
  ...
}

And in enqueueRun, consider rejecting or coalescing when there is already a pending manual run for that jobId (store a per-job pending flag under locked(...)).

2. 🟡 Spoofable cron session keys treated as internal suppress subagent announcement delivery

Property	Value
Severity	Medium
CWE	CWE-807
Location	`src/agents/subagent-announce.ts:82-84`

Description

The subagent announce flow now treats any session key that looks like a cron session as an internal requester:

isInternalAnnounceRequesterSession() returns true when isCronSessionKey(sessionKey) is true.
isCronSessionKey() is purely string/pattern based (any agent:<id>:cron:<...>), and does not verify the session was created by the cron subsystem.
Gateway RPC callers can supply arbitrary sessionKey values in the agent method (validated only for malformed agent: prefix), so a caller can spoof a cron-looking key.
When the requester is considered internal, subagent completion announcements are sent with deliver: false and with channel/to/accountId omitted, suppressing user-visible delivery and changing routing behavior.

Security impact if an untrusted/less-trusted client can call gateway.agent:

Delivery suppression / availability: attacker can prevent subagent completion messages from being delivered externally by choosing a cron-looking sessionKey.
Routing integrity: announcement routing decisions are based on an untrusted string rather than authenticated/authorized cron execution context.

Vulnerable logic (classification):

function isInternalAnnounceRequesterSession(sessionKey: string | undefined): boolean {
  return getSubagentDepthFromSessionStore(sessionKey) >= 1 || isCronSessionKey(sessionKey);
}

Recommendation

Do not rely on string parsing of a client-controllable session key to decide whether a requester is “internal”. Instead, base this on authenticated server-side state.

Recommended approaches:

Mark cron sessions in the session store with non-spoofable metadata (set only by the cron service), then check that metadata:

// When creating cron sessions (cron subsystem only)
await sessions.patch({
  key: cronSessionKey,
  origin: { kind: "cron", jobId },
});

// When deciding internal-ness
function isInternalAnnounceRequesterSession(sessionKey?: string): boolean {
  if (getSubagentDepthFromSessionStore(sessionKey) >= 1) return true;
  const entry = sessionKey ? loadSessionEntryByKey(sessionKey) : undefined;
  return entry?.origin?.kind === "cron";
}

Gate cron-shaped session keys at the gateway boundary: reject or ignore agent:<id>:cron:* session keys from non-admin clients (or from any client except the cron subsystem), e.g. require senderIsOwner / ADMIN scope.
If the intent is to cover only ephemeral cron run sessions, consider using the stricter isCronRunSessionKey() and still pair it with server-side verification.

Any of these ensures cron-only routing behavior cannot be triggered by an untrusted caller choosing a special-looking sessionKey.

Analyzed PR: #40204 at commit 219276e

_{Last updated on: 2026-03-08T22:44:48Z}

greptile-apps · 2026-03-08T19:45:25Z

Greptile Summary

This PR makes three coordinated improvements to the cron delivery pipeline: it prevents subagent announces from bypassing the cron delivery path when the requester is a cron run session; it consolidates descendant fallback output into the cron announce payload when the cron agent's own reply is still an interim placeholder; and it detaches cron.run gateway calls so the API responds immediately after enqueueing background work.

subagent-announce.ts: Adds isInternalAnnounceRequesterSession helper that extends the existing requesterDepth >= 1 subagent guard to also cover cron session keys via isCronSessionKey. The closure captures the let variables requesterDepth and targetRequesterSessionKey by reference, so it correctly re-evaluates after the fallback-traversal mutation at lines 1368–1372.
ops.ts: Extracts prepareManualRun / finishPreparedManualRun from the monolithic run function and adds enqueueRun, which fires finishPreparedManualRun as a detached promise with a logged .catch. The typed PreparedManualRun discriminated union removes the now-dead if (!prepared.executionJob || ...) defensive check. Stale runningAtMs markers from mid-flight termination are handled by the existing startup cleanup.
server-methods/cron.ts: Switches cron.run handler to enqueueRun; response shape is unchanged so existing callers are unaffected.
Test coverage: Three new test cases cover the cron-session-as-requester internal delivery, descendant output consolidation, and immediate gateway return respectively.

Confidence Score: 4/5

Safe to merge; the refactoring is well-scoped and the fire-and-forget pattern is guarded with proper error handling and existing startup cleanup.
The logic changes are small and well-tested. The only non-trivial concern is that the new regression test for cron-session-as-requester relies on subagentSessionRunActive = true (the test-harness default), which may not reflect production behavior if cron sessions are not registered in the subagent run registry — the test does not cover the isSubagentSessionRunActive = false path for cron sessions. Whether that edge case can actually occur depends on whether isolated-agent cron runs create usable session entries, which looks likely from the code structure but is not verified in the test.
src/agents/subagent-announce.timeout.test.ts — the regression test at line 200 should ideally also cover the subagentSessionRunActive = false path for cron sessions to ensure no silent announce failure in that edge case.

Comments Outside Diff (1)

src/agents/subagent-announce.timeout.test.ts, line 200-222 (link)

Regression test relies on subagentSessionRunActive = true default, which may not reflect production behavior for cron sessions

The test at line 200 validates that when requesterSessionKey is a cron session key, the announce is kept internal (deliver=false). However, the test relies on the subagentSessionRunActive = true default set in beforeEach (line 128).

In the runSubagentAnnounceFlow logic (around line 1355), when requesterIsInternalSession() returns true for a cron session key, it enters the block and calls isSubagentSessionRunActive(cronSessionKey). In production, this may return false for cron sessions since they might not be registered as "active subagent runs" in the subagent run registry.

If isSubagentSessionRunActive returns false, the code falls into:
```
const parentSessionEntry = loadSessionEntryByKey(targetRequesterSessionKey);
const parentSessionAlive = hasUsableSessionEntry(parentSessionEntry);
if (!parentSessionAlive) {
  const fallback = resolveRequesterForChildSession(targetRequesterSessionKey);
  if (!fallback?.requesterSessionKey) {
    shouldDeleteChildSession = false;
    return false; // silently fails
  }
}
```
If isolated-agent cron runs do create usable session entries accessible via loadSessionEntryByKey, this path is fine and the announce would still be delivered internally. However, it would be worth confirming this is the case, and potentially adding a variant of this test where subagentSessionRunActive = false to verify the fallback path also keeps the announce internal (or at least doesn't silently drop it).

_{Last reviewed commit: 48b03aa}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce0e11d289

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-08T20:53:05Z

src/agents/subagent-announce.ts

+    const requesterIsInternalSession = () =>
+      requesterDepth >= 1 || isCronSessionKey(targetRequesterSessionKey);


Preserve external notify path for late cron descendants

Treating every cron requester session as internal here means descendant completion announces from cron runs now go through requesterIsSubagent=true, which drives sendSubagentAnnounceDirectly to call agent with deliver=false (internal only). In the active-descendant timeout path, dispatchCronDelivery already returns early without sending (src/cron/isolated-agent/delivery-dispatch.ts:319-330), so those later descendant announces are the only remaining path to user-visible completion; after this change they are suppressed externally, and long-running cron jobs can finish without any final user update.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1d61df4809

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-08T21:33:20Z

src/cron/service/ops.ts

+      "cron: queued manual run background execution failed",
+    );
+  });
+  return { ok: true, enqueued: true, runId } as const;


Return enqueue failures during gateway drain

This path always returns { ok: true, enqueued: true, runId } even when the queue rejects the task immediately. During restart, markGatewayDraining() causes enqueueCommandInLane(...) to reject with GatewayDrainingError, and this code only logs that rejection in the detached .catch, so cron.run reports success while the job is dropped and never executed. Manual runs submitted in that drain window therefore get silently lost from the caller’s perspective.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 219276e940

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-08T21:55:36Z

src/cron/service/ops.ts

+  void enqueueCommandInLane(
+    CommandLane.Cron,
+    async () => {
+      const result = await run(state, id, mode);
+      if (result.ok && "ran" in result && !result.ran) {


Rebind queued cron.run tasks to the current cron service state

enqueueRun captures the current state in a fire-and-forget lane task, so queued manual runs can execute against an old CronServiceState after a hot reload replaces the cron service (src/gateway/server-reload-handlers.ts restarts cron but does not clear CommandLane.Cron). In that window, a request accepted before reload can run with stale config (for example, cronEnabled/store settings from the old state), which can produce runs after operators thought cron had been reconfigured or disabled.

Useful? React with 👍 / 👎.