Skip to content

fix(cron): keep isolated run status ok when delivery fails#95419

Closed
Alix-007 wants to merge 9 commits into
openclaw:mainfrom
Alix-007:fix/cron-isolated-status-not-overwritten-by-delivery
Closed

fix(cron): keep isolated run status ok when delivery fails#95419
Alix-007 wants to merge 9 commits into
openclaw:mainfrom
Alix-007:fix/cron-isolated-status-not-overwritten-by-delivery

Conversation

@Alix-007

@Alix-007 Alix-007 commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Scheduled isolated agentTurn cron jobs could finish a successful isolated session yet have the outer scheduled run persisted with status=error when the post-run delivery phase failed. This patch keeps execution status=ok and records the delivery failure separately, so cron run logs report status=ok with deliveryStatus=not-delivered instead of a false execution failure.

Root cause

In finalizeCronRun (src/cron/isolated-agent/run.ts), when the agent payload was non-fatal (!hasFatalErrorPayload), the dispatch outcome was returned verbatim:

if (!hasFatalErrorPayload || deliveryResult.result.status !== "ok") {
  return resultWithDeliveryMeta; // carries deliveryResult.result.status === "error"
}

A dispatchCronDelivery result of status: "error" (unresolved target / outbound send failure) therefore collapsed into the isolated agent execution status. applyJobResult then set lastRunStatus=error, incremented consecutiveErrors, and could trigger backoff / failure alerts for a session that actually succeeded.

The storage layer already keeps status (CronRunStatus) and deliveryStatus (CronDeliveryStatus) as separate columns; the bug was purely that run.ts contaminated status before persistence.

Changes

  • finalizeCronRun: when the agent payload is non-fatal and the run was not aborted, a delivery dispatch status: "error" now routes through resolveRunOutcome so execution status stays ok. Delivery failure is recorded via delivered/deliveryAttempted and delivery diagnostics, which drive the decoupled run-log deliveryStatus (not-delivered). Aborted runs and fatal agent payloads keep their error status.
  • resolveRunOutcome: carries optional deliveryDiagnostics so the delivery error remains operator-visible in run diagnostics even when status=ok.
  • Adds a regression test asserting a successful final report followed by a delivery dispatcher failure keeps status=ok with delivery failure metadata.

Scope is limited to cron finalization; no provider/channel code is touched.

Decoupling rationale (status vs deliveryStatus)

status answers "did the isolated agent turn execute successfully" and deliveryStatus answers "did the result reach its destination". These are orthogonal: a turn can succeed and the announce/direct send can still fail. Collapsing them made every transient delivery error look like an agent failure and drove backoff/alerting. Keeping them decoupled means a delivery failure is surfaced (deliveryStatus=not-delivered, delivery diagnostics) without falsely failing the execution — avoiding any cross-cutting product-decision change to the scheduler's error semantics.

Semantic change — for maintainer review

This PR changes one observable behavior — the run-log status for the "execution succeeded, delivery failed" path — so I want to lay out the semantics and blast radius explicitly rather than asking you to infer them. This is the only judgement call in the change; the code is otherwise a verbatim re-route.

1. Why status / deliveryStatus decoupling is the correct semantics. An isolated agentTurn is a self-contained unit of work: it either executes successfully and produces a final assistant report, or it does not. Post-run delivery (announce / direct send) is a separate, downstream phase that depends on transient externals (channel resolution, outbound network, provider availability). Today a successful, completed isolated session is retroactively re-labelled status=error purely because the announce send failed afterwards — i.e. work that finished is recorded as failed work. The storage layer already models these as two independent columns (CronRunStatus + CronDeliveryStatus); this PR just stops run.ts from contaminating the first with the second. The user who filed #94058 reached the same conclusion independently — their stated expected behavior is exactly "preserve the successful agent execution outcome and separately record the delivery failure phase."

2. Blast radius on backoff / failure-alert is deliberately narrow. Only one path changes: agent payload non-fatal (!hasFatalErrorPayload) and deliveryResult.result.status === "error" and the run was not aborted (!params.isAborted()). That path goes from status=error to status=ok + deliveryStatus=not-delivered. Everything else is untouched:

  • A genuine execution failure (hasFatalErrorPayload) still records status=errorconsecutiveErrors++ → backoff / failure alerts fire exactly as before.
  • An aborted run still records status=error.
  • A successful delivery still records status=ok + delivered.

So real execution failures continue to drive backoff and alerting; only the false positives (succeeded-but-undelivered) stop doing so. The delivery failure is not swallowed — it remains operator-visible via delivered=false, deliveryAttempted, the deliveryStatus=not-delivered column, and the merged delivery diagnostics.

3. Compatibility note (the merge-risk: compatibility label). The risk to weigh is operators/tooling that currently key off status=error to detect undelivered runs. After this change those runs surface as status=ok + deliveryStatus=not-delivered, so any such consumer should read deliveryStatus (the column already exists for this purpose) rather than overloading status. There is no schema/migration change and no provider/channel code touched. I have kept the code change to the minimum re-route and surfaced the trade-off here so the call on the error-semantics is yours to make — happy to adjust framing or gate the behavior if you'd prefer a more conservative landing.

Real behavior proof

  • Behavior addressed: A successful isolated agentTurn whose post-run delivery fails was persisted to the cron run log with status=error, making a successful session look like a failed run (and feeding error backoff/alerting).

  • Real environment tested: Local OpenClaw source checkout on Linux at upstream/main head 7217477553f64701d2ba36e344ca1cd6be8ba54f, dependencies hydrated, commands run from the repo root.

  • Exact steps or command run after this patch: Drove the REAL CronService finalization (applyJobResult + resolveDeliveryState) for a successful isolated turn with a failing post-run delivery, persisted the resulting finished run record through the REAL appendCronRunLog into a REAL on-disk SQLite database (OPENCLAW_STATE_DIR), then READ IT BACK across the SQLite disk boundary via the REAL readCronRunLogEntriesSync. The injected finalization outcome is exactly what patched run.ts returns, validated independently by the new vitest regression test.

  • Evidence after fix: SQLite disk readback (console-intercept disabled):

    [PROOF after-fix] finished event: {"jobId":"16ede2af-...","status":"ok","delivered":false,"deliveryStatus":"not-delivered"}
    [PROOF after-fix] SQLite readback: {"status":"ok","deliveryStatus":"not-delivered","delivered":false}
    
  • Negative control (pre-fix behavior reproduced): Reverting only run.ts and re-running the regression test reproduces the bug (real run.ts output), and the same SQLite readback flow persists the collapsed status:

    AssertionError: expected 'error' to be 'ok' // Object.is equality   (run.ts regression test, fix reverted)
    
    [PROOF pre-fix] SQLite readback: {"status":"error","deliveryStatus":"not-delivered","delivered":false,"deliveryError":"Message failed"}
    
  • Validation commands (all green after fix):

    • node scripts/run-vitest.mjs src/cron/isolated-agent/run.message-tool-policy.test.ts src/cron/isolated-agent/run.meta-error-status.test.ts src/cron/isolated-agent/delivery-dispatch.double-announce.test.ts src/cron/service.persists-delivered-status.test.ts src/cron/service.runs-one-shot-main-job-disables-it.test.ts -> 166 passed
    • node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental false -> exit 0
    • oxfmt --check on changed files -> all formatted
    • oxlint on changed files -> exit 0

Label: proof: live SQLite run-log readback across disk (status=ok / deliveryStatus=not-delivered) + negative control reproducing the pre-fix collapsed status=error.

AI-assisted.

Fixes #94058

@clawsweeper

clawsweeper Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 5:18 AM ET / 09:18 UTC.

Summary
The PR keeps successful isolated cron runs at status=ok when post-run delivery fails, carries deliveryError separately, and adds regression plus run-log readback tests.

PR surface: Source +59, Tests +167. Total +226 across 6 files.

Reproducibility: yes. Source inspection shows current main can propagate a delivery-dispatch status: "error" after non-fatal isolated execution, and the PR adds negative-control plus SQLite readback proof for the corrected outcome; I did not run a live scheduled chat-provider repro in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Cron Status Semantics: 1 delivery-failure path reclassified. A successful isolated execution followed by delivery failure changes from run error to ok plus deliveryStatus=not-delivered, which affects status consumers before merge.
  • Cron Outcome Surface: 1 optional field added. deliveryError is threaded across runner, service, event, and run-log boundaries, so API/log consumers should see it as a delivery-specific diagnostic.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94058
Summary: This PR is a direct implementation candidate for the narrow isolated cron success/error mismatch; broader warning-status work overlaps but is not a merged replacement.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

Maintainer options:

  1. Land After Accepting Status Contract (recommended)
    Maintainers can accept that delivery-only failures no longer increment execution-error/backoff paths and rely on delivery fields for operator visibility.
  2. Revise Toward Warning Status
    The PR can be reshaped to use the broader warning-status model if maintainers want a distinct partial-success state instead of ok.
  3. Pause Behind Broader Work
    If maintainers do not want parallel partial-success contracts, this PR can wait behind fix: record cron delivery failures as warnings #80139 or its successor.

Next step before merge

  • [P2] There is no concrete automated repair target while the remaining blocker is maintainer acceptance of the cron partial-success status contract.

Maintainer decision needed

  • Question: Should successful isolated cron execution with failed post-run delivery be recorded as status=ok plus deliveryStatus=not-delivered, or should this wait for a first-class warning/partial-success status contract?
  • Rationale: The code path and proof support the narrow fix, but changing persisted run status affects backoff, failure alerts, cron run --wait, UI status, plugin hooks, and external consumers that may currently use status=error for delivery failures.
  • Likely owner: steipete — steipete has the strongest current-history signal across cron status, trigger, and session-target semantics, and the remaining blocker is a product/status contract choice.
  • Options:
    • Accept Narrow Decoupling (recommended): Merge this focused fix so successful execution is not mislabeled as an error while delivery failure remains visible through delivery metadata.
    • Wait For Warning Status: Hold this PR and make the broader warning-status model the canonical partial-success solution.
    • Keep Error Semantics: Reject this direction and continue treating post-run delivery failure as a run error for alerting and compatibility simplicity.

Security
Cleared: The diff is limited to cron runtime status propagation and tests; it does not change dependencies, CI, secrets, permissions, downloads, or supply-chain execution paths.

Review details

Best possible solution:

Land the narrow decoupling only after maintainers accept status=ok plus deliveryStatus=not-delivered, and keep any first-class warning-state work on the broader canonical thread.

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

Yes. Source inspection shows current main can propagate a delivery-dispatch status: "error" after non-fatal isolated execution, and the PR adds negative-control plus SQLite readback proof for the corrected outcome; I did not run a live scheduled chat-provider repro in this read-only review.

Is this the best way to solve the issue?

Yes as a narrow bug fix if maintainers accept the status contract. The safer product alternative is the broader first-class warning status tracked by #49190 and #80139.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 0de5d37f92fc.

Label changes

Label justifications:

  • P2: This is a bounded cron runtime correctness fix with medium operator impact and compatibility-sensitive status semantics.
  • merge-risk: 🚨 compatibility: Merging changes the persisted run status observed by existing scripts, UI, alerts, and cron run --wait for delivery-only failures.
  • merge-risk: 🚨 message-delivery: Merging changes how undelivered cron results are classified and surfaced, so delivery failure visibility depends on delivery metadata instead of run error state.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal proof with real CronService SQLite run-log disk readback plus a negative control reproducing the old collapsed status=error behavior.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal proof with real CronService SQLite run-log disk readback plus a negative control reproducing the old collapsed status=error behavior.
Evidence reviewed

PR surface:

Source +59, Tests +167. Total +226 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 4 62 3 +59
Tests 2 167 0 +167
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 229 3 +226

What I checked:

  • Repository policy read: Root AGENTS.md was read fully and applied; its review policy requires whole-surface review and treats runtime status, persisted state, fallback behavior, and message delivery as compatibility-sensitive. (AGENTS.md:8, 0de5d37f92fc)
  • Current main still has the source-reproducible status collapse: On current main, after delivery dispatch, !hasFatalErrorPayload returns resultWithDeliveryMeta directly, so a delivery-dispatch status: "error" can become the isolated cron run status. (src/cron/isolated-agent/run.ts:1416, 0de5d37f92fc)
  • Current main persists run status into backoff/alert state: applyJobResult writes lastRunStatus from the runner result and increments consecutive error/backoff handling when that status is error. (src/cron/service/timer.ts:727, 0de5d37f92fc)
  • PR head narrows reclassification to non-fatal delivery errors: The PR reclassifies only non-fatal, non-aborted delivery-dispatch errors without errorKind: "delivery-target" through resolveRunOutcome, preserving ok execution status and separate delivery diagnostics. (src/cron/isolated-agent/run.ts:1425, 4fa5f25dcbb0)
  • PR head carries delivery errors across service persistence: The PR threads deliveryError through applyJobResult so lastDeliveryError can be populated without setting the run-level error on successful execution. (src/cron/service/timer.ts:753, 4fa5f25dcbb0)
  • Run-log and plugin surfaces already understand deliveryError: Current main already includes deliveryError in the gateway run-log schema, plugin hook event type, event forwarding, and SQLite run-log readback, so this PR uses an existing public diagnostic field rather than inventing a new protocol key. (packages/gateway-protocol/src/schema/cron.ts:636, 0de5d37f92fc)

Likely related people:

  • steipete: Recent main history shows work on cron event triggers and persistent session targets touching the same finalization/status area, and this PR needs a product/status contract decision rather than a narrow code repair. (role: likely decision owner and recent cron contributor; confidence: high; commits: a6768d9de567, be5906e19dd7, 77f09f25751a; files: src/cron/isolated-agent/run.ts, src/cron/service/timer.ts)
  • vincentkoc: Recent history includes explicit delivery/timeout semantics and dead-code cleanup in cron paths, plus co-authored or reviewed nearby isolated-run behavior changes. (role: recent cron semantics contributor; confidence: medium; commits: efbefceb0e2e, 44ae2fd93601, 0a338147a52f; files: src/cron/isolated-agent/run.ts, src/cron/service/timer.ts)
  • scotthuang: Authored the recent target-session delivery-awareness fix that touched isolated cron delivery metadata and run.ts/delivery-dispatch.ts boundaries adjacent to this PR. (role: adjacent delivery contributor; confidence: medium; commits: 81abc2b21b03; files: src/cron/isolated-agent/run.ts, src/cron/isolated-agent/delivery-dispatch.ts)
  • Alix-007: Besides authoring this PR, this account has prior merged cron timer/backoff work on current main, so they are relevant to the affected scheduler behavior history. (role: recent adjacent cron contributor; confidence: medium; commits: 16fba65cb6ad; files: src/cron/service/timer.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (13 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-06T06:31:37.670Z sha 8a68409 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-06T16:59:51.535Z sha d73b9b3 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-08T10:18:03.689Z sha 2eacae6 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-08T10:27:38.402Z sha 2eacae6 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T01:54:35.370Z sha 678b58a :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T02:06:31.522Z sha 678b58a :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T06:45:59.266Z sha 5055094 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T08:44:05.164Z sha 0829726 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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. labels Jun 20, 2026
@Alix-007

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review — Added a 'Semantic change — for maintainer review' section: only the (non-fatal success + delivery-error + non-aborted) path is reclassified to ok + deliveryStatus:not-delivered; genuine execution failures still record error and trigger backoff/alerts, and delivery failure stays visible via the deliveryStatus column. No schema/provider/channel changes. Fixes #94058.

@clawsweeper

clawsweeper Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 21, 2026
@Alix-007

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review — Fixed a genuine regression the cron-isolated-agent check caught: the #94058 status-decoupling branch was too broad and reclassified deliberate keyless delivery-target denials (errorKind: delivery-target) as ok. Narrowed the condition so those stay error, while transient delivery failures still become ok + deliveryStatus:not-delivered. Local: delivery-awareness 3 pass, full cron/isolated-agent suite 395 tests pass.

@clawsweeper

clawsweeper Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jun 21, 2026
@Alix-007

Copy link
Copy Markdown
Contributor Author

CI note after re-checking the failing check-additional-boundaries-bcd job: the current failure is in the repo-wide lint:plugins:no-extension-test-core-imports boundary check and reports existing extension test imports in extensions/browser/src/browser/cdp-proxy-bypass.test.ts and extensions/memory-core/src/memory/manager-cache.test.ts.

Those files are unrelated to this cron finalizer PR. The cron-specific regression was already narrowed in the latest push so deliberate delivery-target refusals remain error, while transient delivery-send failures become ok with deliveryStatus:not-delivered.

@Alix-007
Alix-007 force-pushed the fix/cron-isolated-status-not-overwritten-by-delivery branch from b570ae5 to cbe8648 Compare June 22, 2026 07:56
@Alix-007
Alix-007 force-pushed the fix/cron-isolated-status-not-overwritten-by-delivery branch from 0ad8b53 to 8a68409 Compare July 6, 2026 06:06
Alix-007 added 3 commits July 7, 2026 00:03
A successful isolated cron agentTurn whose post-run delivery phase returns
an error collapsed the execution status into the delivery error, so the
outer scheduled run persisted status=error even though the isolated session
ended successfully.

Decouple execution status from delivery outcome in finalizeCronRun: when the
agent payload is non-fatal and the run was not aborted, a delivery dispatch
error now keeps status=ok and records the failure separately via
delivered/deliveryAttempted plus delivery diagnostics, which drive the
already-decoupled run-log deliveryStatus (not-delivered). Aborted runs and
fatal agent payloads keep their error status.

Fixes openclaw#94058
The openclaw#94058 fix kept isolated turn status=ok when post-run delivery
fails, but the broad `status === "error"` check also caught the
openclaw#91613 keyless implicit last-target refusal, which is a deliberate
delivery-target guard (errorKind "delivery-target") and must stay
status=error. Exclude that errorKind so genuine delivery dispatch
failures still become ok while target-guard refusals remain errors.
A successful isolated agent turn keeps status=ok when post-run delivery
fails (openclaw#94058), but the ok/not-delivered resolveRunOutcome branch only
forwarded delivery diagnostics and dropped the delivery dispatch error.
That left lastDeliveryError empty and the finished-event deliveryError
field blank, so CLI/UI/API run logs could not show why delivery did not
land for an otherwise successful run.

Thread the delivery error on a dedicated CronRunOutcome.deliveryError
field from resolveRunOutcome through executeDetachedCronJob and
applyJobResult into resolveDeliveryState, so it persists as
lastDeliveryError and reaches the finished event (and thus the run log)
without conflating it with a run-level error (lastError stays empty on a
successful run).

Add a service/run-log readback test proving the delivery error survives
the cron boundary into a persisted run-log entry, and extend the
isolated-turn regression to assert the dedicated deliveryError field.
@Alix-007
Alix-007 force-pushed the fix/cron-isolated-status-not-overwritten-by-delivery branch from 8a68409 to d73b9b3 Compare July 6, 2026 16:03
@Alix-007

Alix-007 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current upstream/main and resolved the manual-run conflict by preserving both behaviors: trigger no-fire/manual-run handling from main, and the isolated-run deliveryError path from this PR.

Local validation on the new head:

  • node scripts/run-vitest.mjs src/cron/isolated-agent/run.message-tool-policy.test.ts src/cron/isolated-agent/run.meta-error-status.test.ts src/cron/isolated-agent/delivery-dispatch.double-announce.test.ts src/cron/service.persists-delivered-status.test.ts src/cron/service.runs-one-shot-main-job-disables-it.test.ts src/cron/service.trigger-eval.test.ts (6 files, 187 tests passed)
  • node_modules/.bin/oxlint src/cron/isolated-agent/run.ts src/cron/isolated-agent/run.message-tool-policy.test.ts src/cron/service.persists-delivered-status.test.ts src/cron/service/ops.ts src/cron/service/timer.ts src/cron/types.ts
  • node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental false

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@Alix-007

Alix-007 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 9, 2026
@steipete

Copy link
Copy Markdown
Contributor

Landed the completed current-main replacement as #105215 / 9bdb2a5ac08deafb6c1feb747530a31b395392a7; #94058 is closed as fixed.

The landed version preserves @Alix-007's contributor commits and completes the invariant across required and best-effort delivery, target-resolution refusal, startup catch-up, manual/removed-job terminal events, diagnostics, persistence, and run-log readback. The original fork history was too stale to update safely without unrelated drift, so the reviewed commits were transplanted to current main and hardened there.

Thank you, @Alix-007.

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

Labels

merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Scheduled isolated agentTurn outer run records error although session ended success

2 participants