Skip to content

fix(cron): wire task-route lease into isolated cron delivery-target (closes #92460, stacked on #95352)#95362

Closed
wangmiao0668000666 wants to merge 8 commits into
openclaw:mainfrom
wangmiao0668000666:fix/92460-task-route-lease
Closed

fix(cron): wire task-route lease into isolated cron delivery-target (closes #92460, stacked on #95352)#95362
wangmiao0668000666 wants to merge 8 commits into
openclaw:mainfrom
wangmiao0668000666:fix/92460-task-route-lease

Conversation

@wangmiao0668000666

@wangmiao0668000666 wangmiao0668000666 commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Isolated cron completion delivery drops explicit delivery.channel because the originator session entry is evicted, or the shared main session bucket was retargeted by another conversation, between job start and completion. Today:

  • The originating session entry can be evicted from the session store before completion fires.
  • The shared agent-main session bucket is last-writer-wins across conversations, so lastChannel/lastTo can point to a different conversation's room when the cron completes.
  • The explicit job.delivery config may not survive to the announce step because resolveDeliveryTarget resolves the target from the session entry, not from job.delivery directly.

The reported case is the worst combination: a manual cron with delivery.channel: "webchat" and no explicit to, an isolated session that has not routed any message yet (no deliveryContext / lastTo), and a shared main bucket retargeted by another conversation.

Closes #92460.

Stacking

This PR is the cron-side wiring of a 2-PR split. The infrastructure half (the generic task_route_leases SQLite module + lifecycle helpers) is in #95352, which should merge first.

PR Branch What Files / Lines Why first
#95352 feat/task-route-lease-module task_route_leases SQLite table + generic lease module + tests + lifecycle repro + executor hooks 7 / +966 Independent, reusable by subagent (#92076) / ACP / codex. No cron coupling.
#95013 (this) fix/92460-task-route-lease Cron acquire / settle / routability / manual runId / GC owner + 4 cron repro scripts 16 / +1721 Depends on #95352's lease API. Closes #92460.

Until #95352 merges, this PR's diff-vs-main shows the union (lease module + cron wiring = 22 files / +2685 lines), because GitHub doesn't allow a fork branch as a PR base. After #95352 lands, this branch will be rebased onto #95352's merge commit, dropping the 3 lease-module commits, and the diff-vs-main will shrink to the 16 cron files (+1721 lines).

Design

A SQLite-backed task_route_leases row keyed by runId, with full lifecycle hooks that fix the reported path across both manual and scheduled cron entry points:

  • Acquire at task start for every cron entry:
    • Scheduled (tryCreateCronTaskRun): explicit acquire, lease keyed by cron:<jobId>:<startedAt> (the internal task ledger id).
    • Manual (tryCreateManualTaskRun): explicit acquire, lease keyed by params.runId when the caller passes one (the manual:... id from enqueueRun), otherwise the same cron: task ledger id as the scheduled path.
  • Forward through the execution pipeline: finishPreparedManualRun passes prepared.runId (the manual: id when present) as the runId arg to executeJobCoreWithTimeout. The pipeline then plumbs it through to runIsolatedAgentJob → resolveCronDeliveryContext → resolveDeliveryTarget → getActiveTaskRouteLease(jobPayload.runId). The resolver's lease lookup hits the same key that tryCreateManualTaskRun acquired the lease under.
  • Post-resolve update after resolveDeliveryTarget returns ok: true (resolveCronDeliveryContext in src/cron/isolated-agent/run.ts calls updateResolvedTaskRouteLease with the resolved (channel, to, threadId, accountId)). Fills in the rest of the target when the cron job's delivery is channel-only (the reported case).
  • Routability-based fallback in resolveDeliveryTarget: the first higher-precedence source that resolves to a routable (channel, to) pair wins; if none is routable, the existing presence-based chain is used so the existing "no target" / "no channel" error path is preserved.
  • Settle on terminal run status:
    • Scheduled (tryFinishCronTaskRun): explicit settle (settled on success/skip, retired on failure).
    • Manual (tryFinishManualTaskRun): explicit settle using the leaseRunId (typically the manual:... id).
  • TTL GC owner: start() arms a dedicated leaseGcTimer (1h cadence, unref) that calls expireStaleTaskRouteLeases. stop() clears it. Runs independently of the cron scheduling timer so subagent/codex/ACP leases are still GC'd when the cron scheduler is disabled.

The auto-settle hook in setDetachedTaskDeliveryStatusByRunId retires the lease on any terminal TaskDeliveryStatus transition (delivered / session_queued → settled; failed → retired) for the generic detached-task path; cron sets deliveryStatus: "not_applicable" which that hook skips, so cron settles explicitly above.

Evidence

The full structured after-fix proof is in the Real behavior proof section below. Quick summary:

  • 51 unit tests passing across 4 test files (32 cron-related + 19 ops.test.ts regression)
  • 24 PASS steps across 4 repro scripts (lifecycle + cron completion + channel-only + manual-cron-lease)
  • Lint exit 0 on touched files
  • Type check (pnpm tsgo:core) exit 0
  • Live behavior: the manual: runId flows through executeJobCoreWithTimeout → runIsolatedAgentJob → resolveDeliveryTarget so the resolver-side getActiveTaskRouteLease(runId) recovers the lease even when the session entry is evicted; the resolver is a routability-aware fallback so the lease is consulted only when higher-precedence sources are unroutable.

Real behavior proof

Behavior addressed: an isolated manual cron with delivery.channel: "webchat" and no explicit to, whose originating session entry was evicted (or whose shared main session bucket was retargeted) before completion fires, still delivers to the resolved target via the task-route lease. The lease is keyed by the manual: run id, and the execution pipeline (executeJobCoreWithTimeout → runIsolatedAgentJob → resolveDeliveryTarget) forwards the same manual: runId to the resolver. Stale leases from process crashes are GC'd within an hour by leaseGcTimer.

Real environment tested: Linux, Node 22.19, real on-disk SQLite via openOpenClawStateDatabase({ env: { OPENCLAW_STATE_DIR: tempDir } }) against fs.mkdtempSync temp dirs. Human ran all repros and tests on a clean checkout.

Exact steps or command run after this patch:

  • pnpm exec tsx scripts/repro/issue-92460-channel-only-empty-session.mts — 7 PASS
  • pnpm exec tsx scripts/repro/issue-92460-cron-completion-leases.mts — 6 PASS
  • pnpm exec tsx scripts/repro/issue-92460-task-route-lease-lifecycle.mts — 7 PASS (covered by feat(tasks): add generic task-route lease module for cross-crash delivery origin recovery #95352; reproduced here as smoke test)
  • pnpm exec tsx scripts/repro/issue-92460-manual-cron-lease.mts — 4 PASS (the reported [Bug]: Isolated cron completion announcer drops explicit delivery.channel on final controller return #92460 shape)
  • node scripts/run-vitest.mjs src/tasks/task-route-lease.test.ts — 13 passed
  • node scripts/run-vitest.mjs src/cron/isolated-agent/delivery-target.issue-92460.test.ts src/cron/isolated-agent/run.issue-91613-preflight.test.ts — 13 passed
  • node scripts/run-vitest.mjs src/cron/service/ops.issue-92460-manual-lease.test.ts — 6 passed
  • node scripts/run-vitest.mjs src/cron/service/ops.test.ts — 19 passed (no regression)
  • node scripts/run-oxlint.mjs on touched files — exit 0

Evidence after fix — manual cron runId alignment (current head):

=== Reproduction for issue #92460 — manual cron runId reaches the resolver ===
State dir: /tmp/openclaw-92460-manual-cron-XXXXXX
Manual run id (caller, from enqueueRun): manual:manual-92460-job:1718726400000:1
Task ledger run id (internal):           cron:manual-92460-job:1718726400000
PASS  1. manual lease acquired under manual:manual-92460-job:1718726400000:1
PASS  2. runIsolatedAgentJob received runId=manual:manual-92460-job:1718726400000:1
PASS  3. resolver-side getActiveTaskRouteLease(runId) recovers the lease under manual: id
PASS  4. terminal settle transitions the manual: lease out of active
ALL PASS

Evidence after fix — channel-only cron + empty session entry:

=== Reproduction for issue #92460 — channel-only cron + empty session entry ===
State dir: /tmp/openclaw-92460-channel-only-XXXXXX
PASS  1. lease acquired at cron job start with channel-only origin (channel=webchat)
PASS  2. lease is unroutable on its own (no `to`) — needs resolver to fill in the target
PASS  3. resolver updated the lease with the resolved target (to=user:resolved-requester, threadId=thread-42)
PASS  4. lease is now routable (channel + to + threadId)
PASS  5. completion-time lookup recovers the resolved target from the lease
PASS  6. terminal settle transitions the lease out of active
PASS  7. lifecycle is repeatable across cron runs
ALL PASS

Observed result after fix: every cron entry path (manual + scheduled + startup catch-up) acquires a lease keyed by the run id the resolver will look up; the resolver consults the lease as a routability-aware fallback; the manual runId flows through executeJobCoreWithTimeout → runIsolatedAgentJob → resolver; leases are settled on terminal status; expireStaleTaskRouteLeases runs hourly via start()'s leaseGcTimer.

What was not tested: no live Telegram / webchat channel send path is exercised in this PR (gated by the resolver's ok: true return, which is proved by the unit tests). The lease-only path is proved end-to-end against real on-disk SQLite with a runIsolatedAgentJob mock that records the runId arg.

Verification

  • pnpm tsgo:core — exit 0
  • node scripts/run-oxlint.mjs on touched files — exit 0
  • 51 unit tests passing across 4 test files (32 cron-related + 19 ops.test.ts regression)
  • 24 PASS steps across 4 repro scripts

Commits

The branch fix/92460-task-route-lease is stacked on feat/task-route-lease-module (PR #95352):

6a7164f617 test(cron): move manual lease reset/close under temp state env
4512fd43d8 fix(cron): forward manual runId through executeJobCoreWithTimeout
224e2e9072 fix(cron): wire task-route lease into manual cron path + GC owner
91552d2f2d fix(cron): isolate lease test fixtures + lint fixes
469227b604 fix(cron): route-lease resolver uses routability + post-resolve update
5a60cb83f0 fix(cron): wire task-route lease into isolated cron delivery-target
9171578c8c test(tasks): isolate lease test fixtures                     [in PR #95352]
d50455ebc3 feat(tasks): add updateTaskRouteLease helper to lease API   [in PR #95352]
2387731314 feat(tasks): add task-route lease module                    [in PR #95352]
a42a1af942 fix(openrouter): bound oauth error bodies                   [base]

Related

AI assistance

AI-assisted (Claude Code). All repro scripts and unit tests were run by the human on a clean checkout of the branch.

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime scripts Repository scripts size: XL labels Jun 20, 2026
@clawsweeper

clawsweeper Bot commented Jun 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 21, 2026, 1:48 PM ET / 17:48 UTC.

Summary
The PR adds task-route lease storage plus cron acquire, resolve, settle, manual runId plumbing, GC, repro scripts, and tests for isolated cron completion delivery.

PR surface: Source +800, Tests +1121, Generated +44, Other +706. Total +2671 across 22 files.

Reproducibility: yes. source-level: current main allows same raw runId across scoped task records, while this PR stores and resolves route leases by raw run_id alone. The PR body also includes real SQLite terminal repro output for the intended cron path, but not live webchat delivery proof.

Review metrics: 3 noteworthy metrics.

  • Persisted Route State: 1 table, 3 indexes added. The diff adds requester-origin routing data to shared SQLite, so storage identity and retention are review-critical.
  • Lease Key Shape: 1-column primary key on run_id. Current main already treats raw runId as ambiguous across task scopes, so this measured key shape is a concrete merge blocker.
  • Foundation Dependency: 1 open replacement foundation PR. The current branch is stacked on a closed foundation while the active replacement foundation uses a different scoped API contract.

Stored data model
Persistent data-model change detected: database schema: src/state/openclaw-state-schema.generated.ts, database schema: src/state/openclaw-state-schema.sql, serialized state: src/cron/service/state.ts, serialized state: src/state/openclaw-state-db.generated.d.ts, serialized state: src/state/openclaw-state-schema.generated.ts, serialized state: src/state/openclaw-state-schema.sql. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #92460
Summary: This PR is a candidate cron-side fix for the canonical isolated-cron completion-origin issue, but the lease foundation is now tracked by an open replacement PR.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • Rebase onto the scoped task-route lease foundation and remove the raw-runId table/API from this branch.
  • Pass cron lease scope consistently through acquire, resolver lookup/update, and settle paths with same-runId collision coverage.
  • [P1] Add redacted live webchat or equivalent transport proof showing the isolated cron completion reaches the original conversation.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR includes terminal output from real on-disk SQLite repro scripts, but it explicitly does not show live webchat or equivalent transport completion reaching the intended conversation after the routing change. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A visible real-transport proof would materially help reviewers verify that the route fix delivers completion to the intended conversation. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify an isolated manual cron with delivery.channel webchat and no explicit to delivers its completion to the original conversation after the session entry is evicted.

Risk before merge

  • [P1] The PR adds durable requester-origin routing state in shared SQLite, so schema identity, cleanup, and upgrade behavior need maintainer acceptance before merge.
  • [P1] The current branch can overwrite, read, or settle another runtime/session scope's route when detached tasks share a raw runId, causing wrong-target or missing completion delivery.
  • [P1] The user-visible live webchat or equivalent transport completion path remains unproven; the PR body explicitly says no live channel send path was exercised.

Maintainer options:

  1. Rebase Onto Scoped Lease Foundation (recommended)
    Use the open scoped foundation as the base, remove the raw-runId table/API from this branch, and wire cron acquire, lookup, update, and settle through the accepted scope tuple before merge.
  2. Declare Cron-Only Global Run IDs
    Maintainers could instead narrow this PR to a cron-only route identity, but that needs explicit docs/tests proving cron run ids cannot collide across requester scopes.
  3. Pause Until Foundation Lands
    Keep this PR open but wait for the foundation PR to merge or be rejected, then re-review a smaller cron-only diff against the final lease contract.

Next step before merge

  • [P1] Manual review is needed because the branch depends on an unsettled persistent route-state foundation and still has raw-runId/session-state blockers that should be resolved against the canonical foundation.

Security
Needs attention: The raw runId route lease can cross requester scopes and expose or reuse another task's delivery origin, which is a concrete wrong-target delivery risk.

Review findings

  • [P1] Scope route leases beyond raw runId — src/tasks/task-route-lease.ts:140-146
  • [P2] Delete route leases when tasks are pruned — src/tasks/task-route-lease.ts:324-336
  • [P3] Clear settled_at on lease re-acquire — src/tasks/task-route-lease.ts:140-146
Review details

Best possible solution:

Land or accept the scoped task-route lease foundation first, then rebase this cron wiring onto that API, pass the matching scope through acquire/update/lookup/settle, and add live transport proof for the isolated-cron completion path.

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

Yes, source-level: current main allows same raw runId across scoped task records, while this PR stores and resolves route leases by raw run_id alone. The PR body also includes real SQLite terminal repro output for the intended cron path, but not live webchat delivery proof.

Is this the best way to solve the issue?

No as written. A route lease is the right general repair direction, but this PR needs the scoped lease foundation and cron-scope plumbing before it is the narrow maintainable fix.

Full review comments:

  • [P1] Scope route leases beyond raw runId — src/tasks/task-route-lease.ts:140-146
    Current main deliberately allows the same raw runId to exist in multiple task scopes. This PR keys, reads, updates, and settles task_route_leases by run_id alone, so one runtime/session scope can overwrite or consume another scope's requester origin and deliver completion output to the wrong target. Rebase onto the scoped lease API or include the same runtime/scopeKind/ownerKey/childSessionKey tuple here with cross-scope coverage.
    Confidence: 0.92
  • [P2] Delete route leases when tasks are pruned — src/tasks/task-route-lease.ts:324-336
    The added GC helper only marks active expired rows as expired; settled, retired, expired, and orphaned rows are never hard-deleted, and current task cleanup deletes only task_delivery_state and task_runs. Add a task-delete cascade or another bounded production cleanup path so durable requester-route rows do not accumulate indefinitely.
    Confidence: 0.86
  • [P3] Clear settled_at on lease re-acquire — src/tasks/task-route-lease.ts:140-146
    The conflict update can make a settled row active again, but it does not clear the old settled_at value. Reset settled_at to null during re-acquire and add a regression assertion for active-after-reacquire leases so callers do not see stale terminal metadata on an active lease.
    Confidence: 0.82

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: This PR targets a real P1 cron completion-delivery loss bug, but the current patch can still misroute or suppress completions through raw runId route identity.
  • merge-risk: 🚨 compatibility: The diff adds a durable SQLite table and route lifecycle whose key shape and cleanup semantics affect upgrades and persisted state.
  • merge-risk: 🚨 session-state: The PR persists requester-origin session routing outside the existing scoped task identity model.
  • merge-risk: 🚨 message-delivery: The changed fallback target selection can recover the wrong delivery origin, no origin, or a stale origin when raw runIds collide.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR includes terminal output from real on-disk SQLite repro scripts, but it explicitly does not show live webchat or equivalent transport completion reaching the intended conversation after the routing change. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +800, Tests +1121, Generated +44, Other +706. Total +2671 across 22 files.

View PR surface stats
Area Files Added Removed Net
Source 12 814 14 +800
Tests 4 1121 0 +1121
Docs 0 0 0 0
Config 0 0 0 0
Generated 2 44 0 +44
Other 4 706 0 +706
Total 22 2685 14 +2671

Security concerns:

  • [high] Unscoped lease key can expose requester routes — src/tasks/task-route-lease.ts:140
    requester_origin_json is persisted and resolved by raw run_id only even though current task state scopes same-runId records; a colliding task can replace or recover another task's route and send sensitive completion text to the wrong conversation.
    Confidence: 0.87

What I checked:

  • Repository policy read: Root AGENTS.md and the relevant scripts/gateway scoped guides were read; the review applied the repository's compatibility, stored-state, session-state, message-delivery, and PR proof rules. (AGENTS.md:1, eea777c9fc9c)
  • Current main uses scoped task run identity: Current main scopes run-id operations by runtime, scopeKind, ownerKey, and childSessionKey and returns no unscoped matches when a runId spans multiple scopes. (src/tasks/task-registry.ts:728, eea777c9fc9c)
  • Current tests require same-runId isolation: Current main has regression coverage that keeps distinct task records when different producers share the same raw runId and scopes lifecycle events to the matching session. (src/tasks/task-registry.test.ts:1832, eea777c9fc9c)
  • PR lease storage is raw-runId keyed: The PR's task_route_leases schema uses run_id as the sole primary key, which conflicts with current main's scoped same-runId task invariant. (src/state/openclaw-state-schema.sql:1153, 9eb248b01bc5)
  • PR lease API overwrites by raw runId: acquireTaskRouteLease upserts on run_id alone, and getActiveTaskRouteLease / settleTaskRouteLease also query by run_id alone, so one scope can replace or settle another scope's route. (src/tasks/task-route-lease.ts:140, 9eb248b01bc5)
  • Replacement foundation fixes the key shape: The open replacement foundation PR uses a composite primary key on run_id, runtime, scope_kind, owner_key, and child_session_key and scoped API lookups, so this cron PR should be rebased to that contract before merge. (src/state/openclaw-state-schema.sql:1143, 319bef76bf1f)

Likely related people:

  • vincentkoc: Current blame for the scoped run-id invariant, task cleanup path, and cron delivery resolver points to Vincent Koc, and the issue/PR discussion routes the generic route-lease design for his review. (role: recent area contributor and review-context owner; confidence: high; commits: 9f888d95e082, 338d31304314, 80ed55332d77; files: src/tasks/task-registry.ts, src/tasks/task-registry.store.sqlite.ts, src/cron/isolated-agent/delivery-target.ts)
  • steipete: Git history shows Peter Steinberger repeatedly touched the shared state/schema, task persistence, and cron/runtime seams adjacent to this new durable route table. (role: adjacent storage and runtime contributor; confidence: medium; commits: 4fa961d4f149, df525b90f29f, 5fdde9b237fa; files: src/state/openclaw-state-schema.sql, src/tasks/task-registry.store.sqlite.ts, src/cron/isolated-agent/run.ts)
  • Shakker: Cron history shows Shakker carried several refactors around delivery target runtime seams, session store loading, and isolated cron runtime boundaries that this PR builds on. (role: recent cron delivery seam contributor; confidence: medium; commits: 24edb82eced9, b98ee0181430, de952c036abc; files: src/cron/isolated-agent/delivery-target.ts, src/cron/isolated-agent/run.ts, src/cron/service/ops.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.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. 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. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 20, 2026
This was referenced Jun 20, 2026
Closed
Open
@wangmiao0668000666
wangmiao0668000666 force-pushed the fix/92460-task-route-lease branch from 6a7164f to 9eb248b Compare June 21, 2026 03:00
@wangmiao0668000666 wangmiao0668000666 mentioned this pull request Jun 21, 2026
Closed
@clawsweeper clawsweeper Bot added status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. labels Jun 21, 2026
@wangmiao0668000666

wangmiao0668000666 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Closing this PR in favor of the scoped foundation in #95481.

The P1 blocker (not just "needs more proof"):

ClawSweeper flagged that this branch keys task_route_leases by raw run_id
alone (src/state/openclaw-state-schema.sql:1153, src/tasks/task-route-lease.ts:140),
but current openclaw/main deliberately allows the same raw runId to exist in
multiple task scopes (runtime / scopeKind / ownerKey / childSessionKey). That
shape lets one task scope overwrite or consume another scope's route lease
and deliver completion output to the wrong conversation — a concrete
wrong-target delivery risk, not a missing-proof issue.

"Unscoped lease key can expose requester routes — requester_origin_json is
persisted and resolved by raw run_id only even though current task state
scopes same-runId records; a colliding task can replace or recover another
task's route and send sensitive completion text to the wrong conversation."
— ClawSweeper review, confidence 0.87

The P2 (no hard-delete GC for settled/expired rows) and P3 (re-acquire does
not clear settled_at) follow from the same shape mistake and are out of
scope until the key is fixed.

Why I closed without going wider:

This PR is stacked on the now-closed foundation #95352. The active replacement
foundation is #95481 (feat/task-route-lease-module-v2), which uses a
composite primary key on (run_id, runtime, scope_kind, owner_key, child_session_key) and is already at status: 👀 ready for maintainer look
with proof: sufficient (🦞 diamond lobster proof, 🐚 platinum hermit patch).
Trying to widen this branch into a foundation rewrite would duplicate #95481
and reopen the same review cycle — worse code quality and worse reviewer
experience than just landing the foundation first.

The follow-up shape (when #95481 lands):

A new PR (working name: #95362-v2) will:

  1. Rebase onto the merged feat(tasks): add generic task-route lease module for cross-crash delivery origin recovery #95481 foundation (composite key API only).
  2. Pass the matching scope tuple through cron acquire, resolver lookup,
    update, and settle paths so cron wiring inherits the scoped contract
    by construction.
  3. Add cross-scope same-runId coverage (mirroring the task-executor.test.ts
    pattern in feat(tasks): add generic task-route lease module for cross-crash delivery origin recovery #95481 at src/tasks/task-executor.test.ts:825).
  4. Add live webchat delivery proof for the isolated cron completion path
    (Mantis / manual Telegram channel round-trip) — ClawSweeper called this
    out as the missing live transport proof for this cluster.

Thanks:

Thanks to ClawSweeper for the precise P1 finding (file + line + confidence)
and to @vincentkoc for the early scope-tuple direction in the #92460 thread
— that's what made the foundation PR land clean.

Closing PR: #95362

@wangmiao0668000666

wangmiao0668000666 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of scoped foundation #95481; see comment for full rationale: #95362 (comment)

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

Reviewers

No reviews

Assignees

No one assigned

Labels

gateway Gateway runtime 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. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. scripts Repository scripts size: XL status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Isolated cron completion announcer drops explicit delivery.channel on final controller return

1 participant

@wangmiao0668000666