Skip to content

fix: record cron delivery failures as warnings#80139

Open
HemantSudarshan wants to merge 3 commits into
openclaw:mainfrom
HemantSudarshan:hemant/49190-cron-warning-status
Open

fix: record cron delivery failures as warnings#80139
HemantSudarshan wants to merge 3 commits into
openclaw:mainfrom
HemantSudarshan:hemant/49190-cron-warning-status

Conversation

@HemantSudarshan

@HemantSudarshan HemantSudarshan commented May 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #49190.

Problem summary

Cron isolated-agent runs currently collapse final delivery failures into the primary error run status even when the agent execution itself succeeded. That makes notification/channel failures look like execution failures, increments error/backoff state, can mark task records failed, and leaves the Control UI without a first-class status filter for this partial-success case.

Root cause

The isolated delivery dispatcher returned status: "error" for unresolved delivery targets and direct outbound send failures. applyJobResult only distinguished ok, skipped, and error, so delivery-only failures used the same scheduler path as model/tool/runtime failures.

Architectural reasoning

This keeps the change additive and follows the existing cron contract surface:

  • Adds warning to the cron run status union, gateway validation schema, run-log filters, plugin hook type, CLI/UI mirrors, and QA helper wait logic.
  • Leaves real agent/runtime failures as error; finalizeCronRun only asks delivery dispatch to downgrade delivery failures to warning when the agent payload did not already indicate a fatal execution failure.
  • Keeps delivery diagnostics in the delivery-specific fields: warning runs set delivered: false, persist lastDeliveryStatus: "not-delivered", and keep lastDeliveryError.
  • Keeps scheduler behavior aligned with partial success: warning runs reset execution-error counters, do not trigger error backoff/failure alerts, complete task-ledger records as successful execution, and terminal one-shot warning runs are disabled but not auto-deleted so operators can inspect state.
  • Extends Control UI filters/status labels and CLI coloring without changing existing ok, error, or skipped semantics.

Real behavior proof

  • Behavior or issue addressed: Cron isolated-agent runs whose agent execution completes but final announce delivery cannot resolve/send now persist as warning instead of error.
  • Real environment tested: Windows 11 local OpenClaw source checkout on branch hemant/49190-cron-warning-status after rebasing onto upstream/main (6e18c8ec15). Runtime used Node 24.14.0 through node --import tsx; no live Slack credentials or private endpoints were used.
  • Exact steps or command run after this patch: Ran a one-off local Node script from the repo root with node --import tsx --input-type=module. The script loaded the real CronService and dispatchCronDelivery modules, created an isolated cron job with Slack announce delivery, simulated the delivery target failure outbound not configured for channel slack, ran the job with cron.run(job.id, "force"), and printed the persisted job state plus dispatcher result.
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output): Copied terminal output from the local run:
{
  "proof": "cron delivery warning status after patch",
  "manualRunOk": true,
  "jobState": {
    "enabled": false,
    "lastRunStatus": "warning",
    "lastStatus": "warning",
    "consecutiveErrors": 0,
    "lastDelivered": false,
    "lastDeliveryStatus": "not-delivered",
    "lastDeliveryError": "outbound not configured for channel slack"
  },
  "finishedEvent": {
    "status": "warning",
    "delivered": false,
    "deliveryStatus": "not-delivered",
    "deliveryError": "outbound not configured for channel slack"
  },
  "dispatchResult": {
    "status": "warning",
    "errorKind": "delivery-target",
    "delivered": false,
    "deliveryAttempted": false,
    "error": "outbound not configured for channel slack"
  }
}
  • Observed result after fix: The cron run completed with manualRunOk: true, persisted lastRunStatus: "warning", kept consecutiveErrors: 0, marked delivery as not-delivered, and returned the dispatcher delivery-target failure as warning with delivered: false.
  • What was not tested: No live Slack workspace send was attempted because this proof intentionally used a redacted local setup without channel credentials. Live channel sends remain covered by existing channel runtime paths; this PR only changes the status classification after delivery failure.

Testing performed

  • pnpm test src/cron/isolated-agent/delivery-dispatch.double-announce.test.ts src/cron/service.persists-delivered-status.test.ts src/cron/service/timer.regression.test.ts src/gateway/protocol/cron-validators.test.ts ui/src/ui/views/cron.test.ts ui/src/ui/controllers/cron-filters.test.ts src/tasks/task-registry.test.ts
  • pnpm test src/cron/service.persists-delivered-status.test.ts src/cron/run-log.test.ts -- -t "persists warning|filters warning"
  • pnpm test src/cron/cron-protocol-conformance.test.ts src/cron/cron-protocol-schema.test.ts
  • pnpm lint:core
  • pnpm lint:scripts
  • pnpm lint:extensions
  • pnpm lint:ui:no-raw-window-open
  • pnpm docs:check-mdx
  • pnpm lint:docs -- docs/cli/cron.md docs/automation/cron-jobs.md
  • pnpm exec oxfmt --check --threads=1 ...changed files...
  • pnpm tsgo:core
  • pnpm tsgo:core:test
  • pnpm tsgo:test:ui
  • pnpm tsgo:extensions
  • pnpm tsgo:core (after rebasing onto latest upstream/main, to verify the OpenAI stream type CI repair)
  • pnpm tsgo:extensions:test
  • pnpm check:madge-import-cycles (after rebasing onto latest upstream/main)
  • pnpm test src/cron/run-log.test.ts -- --reporter=dot
  • pnpm test src/cron/run-log.test.ts src/cron/service.persists-delivered-status.test.ts src/cron/service/timer.regression.test.ts src/cron/isolated-agent/delivery-dispatch.double-announce.test.ts src/gateway/protocol/cron-validators.test.ts ui/src/ui/controllers/cron-filters.test.ts ui/src/ui/views/cron.test.ts src/agents/openai-transport-stream.test.ts -- --reporter=dot
  • pnpm exec oxfmt --check --threads=1 src/cron/run-log.ts src/agents/openai-transport-stream.ts- pnpm ui:i18n:check (after rebasing onto latest upstream/main; local Windows run reached translation subprocess spawn EINVAL after raw-copy baseline passed)
  • pnpm check:changed
  • git diff --check

Local note: the full pnpm test src/cron/run-log.test.ts -- --reporter=dot initially reproduced a Windows append-path failure from appendRegularFile (Refusing to append after file changed). I fixed the run-log writer to keep the symlink-parent guard while appending through the fs-safe root writer, then reran pnpm test src/cron/run-log.test.ts -- --reporter=dot and the grouped cron/OpenAI targeted command successfully. Also, ui:i18n:sync initially failed because the configured translation subprocess could not spawn in this Windows shell; I reran the repo sync/check path with translation keys falling back to English for the new cron.runs.runStatusWarning key, which updated the locale metadata fallback keys and passed control-ui-i18n check.

Risk analysis

Risk is medium-low. The protocol change is additive, existing statuses keep their behavior, and the scheduler continues to apply error retry/backoff only to error. The main compatibility surface is clients that exhaustively switch on cron run statuses; this PR updates repo-owned TS/UI/plugin helper surfaces and leaves generated Swift/openclaw-kit fields as AnyCodable/string-compatible where applicable.

Backward compatibility

Existing stored ok, error, and skipped run records remain valid. New warning records are accepted by the gateway schema, CLI/UI type mirrors, run-log filters, and plugin hooks. Older external clients that do not know warning will see a new string status but can still inspect deliveryStatus/deliveryError for details.

Screenshots/logs

No screenshots; this is a scheduler/protocol/UI-state change. Relevant command logs are summarized above.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation app: web-ui App: web-ui gateway Gateway runtime cli CLI command changes scripts Repository scripts agents Agent runtime and tooling extensions: qa-lab size: L proof: supplied External PR includes structured after-fix real behavior proof. labels May 10, 2026
@clawsweeper

clawsweeper Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: current main still has the cron delivery/status problem, but the clean, proof-positive path is now #95419, while this broader warning-status branch is conflicting and has a concrete QA wait regression.

Root-cause cluster
Relationship: superseded
Canonical: #95419
Summary: This PR and the replacement PR both address successful isolated cron execution being treated as a delivery failure/error, but the replacement is the clean, proof-positive, mergeable path while this branch is conflicting and broader.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Close this conflicting branch and continue the cron delivery-status semantics decision on #95419.

So I’m closing this here and keeping the remaining discussion on #95419.

Review details

Best possible solution:

Close this conflicting branch and continue the cron delivery-status semantics decision on #95419.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible: current main has only ok, error, and skipped cron run statuses, and the isolated finalizer can still return an error-shaped delivery result as the outer run result. I did not run a live cron delivery scenario in this read-only pass.

Is this the best way to solve the issue?

No, not as the best landing path now. A first-class warning status is a plausible product direction, but this branch is conflicting, has a QA wait bug, and is superseded by the cleaner mergeable delivery-status PR.

Security review:

Security review cleared: No concrete security or supply-chain regression was found; the diff adds no dependencies, workflows, permissions, secrets handling, install hooks, or third-party code execution.

AGENTS.md: found and applied where relevant.

What I checked:

  • Live PR state: GitHub reports this PR open, non-draft, maintainer-editable, and conflicting against the current base. (6e18c8ec1591)
  • Current-main status contract: Current main still defines cron execution status as ok, error, or skipped, so the warning-status request is not already implemented on main. (src/cron/types.ts:95, 9d68f877ac3e)
  • Current-main collapse path: Current main returns resultWithDeliveryMeta when a nonfatal isolated run receives an error-shaped delivery result, so delivery failure can still become the outer cron run status. (src/cron/isolated-agent/run.ts:1368, 9d68f877ac3e)
  • Viable canonical replacement: The narrower replacement PR is open, non-draft, mergeable clean, proof-sufficient, and rated ready for maintainer look; its latest review frames the remaining blocker as maintainer semantics choice rather than a contributor repair. (0b88da287bea)
  • Patch defect in superseded branch: The PR head only applies the afterTs guard to ok; older error, skipped, or warning entries can be treated as the new run completion. (extensions/qa-lab/src/cron-run-wait.ts:45, 6e18c8ec1591)
  • Maintainer notes check: No .agents/maintainer-notes/ directory exists in this checkout, so there were no matching internal maintainer notes to apply. (9d68f877ac3e)

Likely related people:

  • steipete: Recent GitHub commit history shows major cron delivery, session-target, and schema/docs work on the execution-versus-delivery boundary affected by this PR. (role: feature history owner; confidence: high; commits: be5906e19dd7, 93e75f646fff, 4c613fbfe05f; files: src/cron/isolated-agent/run.ts, src/cron/isolated-agent/delivery-dispatch.ts, src/cron/service/timer.ts)
  • vincentkoc: History shows recent work and review/co-author activity across isolated cron finalization, explicit delivery semantics, scheduler state, and protocol-adjacent cron surfaces. (role: recent area contributor; confidence: high; commits: efbefceb0e2e, e573b751bf4b, 0a338147a52f; files: src/cron/isolated-agent/run.ts, src/cron/isolated-agent/delivery-dispatch.ts, src/cron/service/timer.ts)
  • shakkernerd: History shows adjacent cron delivery/cancellation work and commits around cron finalization and delivery behavior. (role: adjacent refactor owner; confidence: medium; commits: 4eb4b87c8ede, 9082233a433c, 24196e05f59b; files: src/cron/isolated-agent/run.ts, src/cron/isolated-agent/delivery-dispatch.ts, src/cron/service/timer.ts)
  • Alix-007: This person authored the current clean canonical PR and has a recent merged cron scheduler/backoff fix in the same status/backoff area. (role: adjacent fix owner; confidence: medium; commits: 16fba65cb6ad, 0b88da287bea; files: src/cron/service/timer.ts, src/cron/isolated-agent/run.ts, src/cron/types.ts)

Codex review notes: model internal, reasoning high; reviewed against 9d68f877ac3e.

@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 1, 2026
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. labels Jun 1, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 2, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jun 14, 2026
@clawsweeper clawsweeper Bot added the rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. label Jun 15, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. label Jun 22, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 4, 2026
@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 19, 2026
@clawsweeper

clawsweeper Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix: record cron delivery failures as warnings This is item 1/1 in the current shard. Shard 3/4.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling app: web-ui App: web-ui cli CLI command changes docs Improvements or additions to documentation extensions: qa-lab gateway Gateway runtime merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. scripts Repository scripts size: L stale Marked as stale due to inactivity status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Cron should distinguish execution success from delivery failure and show warning instead of error

1 participant