Codex review: needs real behavior proof before merge. Reviewed June 20, 2026, 11:34 AM ET / 15:34 UTC.

Summary
The PR adds a SQLite-backed task-route lease table/API and wires cron acquire, settle, resolver fallback/update, manual runId propagation, GC, repro scripts, and regression tests to preserve isolated cron completion delivery origin.

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

Reproducibility: yes. source-level. Current main tests intentionally permit colliding runIds across requester scopes, and the PR key/lookup uses only raw runId; I did not run the PR's repro scripts in this read-only review.

Review metrics: 1 noteworthy metric.

• Persisted Route State: 1 SQLite table added. The PR persists requester delivery origins in shared state, so key, retention, cleanup, and upgrade semantics matter before merge.

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 the current candidate fix for the isolated cron completion delivery issue, while the open foundation PR owns overlapping lease infrastructure needed by this branch.

Members:

• canonical: [Bug]: Isolated cron completion announcer drops explicit delivery.channel on final controller return #92460 - The PR body uses closing syntax for this open bug and the diff targets its isolated cron completion delivery failure.
• partial_overlap: feat(tasks): add generic task-route lease module for cross-crash delivery origin recovery #95352 - The open stacked PR owns the generic task-route lease table/API that this PR currently includes in its diff until rebased.
• adjacent_distinct: Subagent completion delivery can fail when requester run is inactive and session transcript is locked #92076 - The issue concerns subagent completion delivery and may reuse a scoped lease module later, but it is not the same cron completion bug.

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:

• [P1] Scope the lease key/lookup and add a cross-scope same-runId regression test.
• [P1] Provide redacted real webchat or transport proof after the scoped-identity fix.
• Rebase this PR after the foundation PR lands or keep this branch limited to cron wiring.

Proof guidance:

• [P1] Needs stronger real behavior proof before merge: The PR supplies terminal output and real SQLite repros, but it does not prove a real webchat/transport completion after the scoped-route issue is fixed; after adding redacted proof, updating the PR body should trigger re-review. 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 browser-visible webchat proof would materially improve confidence that the isolated cron completion reaches the intended conversation after the route fix. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

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

Risk before merge

• [P1] A raw runId-only lease can overwrite or expose another runtime/requester scope's requester origin when two detached tasks share a runId, causing wrong-target or missing completion delivery.
• [P1] The new persistent SQLite route state needs maintainer-approved key, retention, cleanup, and upgrade semantics before merge; the smaller foundation PR is still open.
• [P1] The supplied proof does not yet show an actual webchat or transport completion delivery after the scoped-identity issue is fixed.

Maintainer options:

1. Scope The Lease Identity (recommended)
   Change the lease key and lookup to include the same runtime/requester scope facts current task registry uses, then add a regression where colliding runIds cannot read or overwrite each other's route.
2. Accept A New Global runId Contract
   Maintainers could explicitly decide that detached runId is now globally unique, but that would require updating current task-registry invariants, tests, and upgrade expectations before this PR lands.
3. Pause Until Foundation Lands
   Because the PR is stacked, maintainers can land or revise feat(tasks): add generic task-route lease module for cross-crash delivery origin recovery #95352 first and keep this branch as cron-only wiring after the shared lease API is settled.

Next step before merge

• [P1] Manual review is needed because the blocker is the stored route identity and stacked foundation API direction, not a safe one-shot autonomous repair.

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
• [P3] Remove the dead task-completion docs reference — src/state/openclaw-state-schema.sql:1146

@clawsweeper re-review

Both P1 findings from the first cut are addressed in this revision:

• P1 fix: add @lid format support and allowFrom wildcard handling #1 (routability, not presence) — resolveDeliveryTarget now prefers the first higher-precedence source that resolves to a routable (channel, to) pair. An empty isolated-cron session entry no longer masks the lease. New helper isRoutableDeliveryEntry + routableCandidate lookup in src/cron/isolated-agent/delivery-target.ts.
• P1 Login fails with 'WebSocket Error (socket hang up)' ECONNRESET #2 (capture the resolved target, not raw job.delivery) — resolveCronDeliveryContext now updates the lease with the resolver's concrete (channel, to, threadId, accountId) after resolveDeliveryTarget returns ok: true. New public function updateTaskRouteLease in src/tasks/task-route-lease.ts + caller in src/cron/isolated-agent/run.ts.

New unit tests: 13 lease tests (was 9) + 10 cron delivery-target tests (was 5) = +9 new tests, all passing.

New repro: scripts/repro/issue-92460-channel-only-empty-session.mts exercises the exact reported path (cron with delivery.channel: "webchat" and no explicit to, empty session entry) and demonstrates the lease acquires with channel-only origin, the resolver updates it with the resolved target, the completion-time lookup recovers the full target, and the lifecycle repeats across cron runs. 7/7 PASS.

PR body updated with the new evidence in the same single-line Field name: value format.

🦞🧹
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 re-review

Two P2 findings from the merge-readiness review are fixed in c1b4939:

P2 (test isolation, both files) — src/tasks/task-route-lease.test.ts and src/cron/isolated-agent/delivery-target.issue-92460.test.ts previously called resetTaskRouteLeasesForTests() (which hits the default shared state DB) before any temp OPENCLAW_STATE_DIR was installed. Both beforeEach hooks now open a fresh temp state DB via openOpenClawStateDatabase({ env: { OPENCLAW_STATE_DIR: createTempStateDir() } }), so lease helpers operate strictly on the test fixture. resetTaskRouteLeasesForTests() calls and the import have been dropped from both suites; closeOpenClawStateDatabaseForTest() is used in afterEach and the process.on("exit") guard.

P2 (existing mock missing new export) — src/cron/isolated-agent/run.issue-91613-preflight.test.ts mocked ./run-delivery.runtime.js with only resolveDeliveryTarget, so resolveCronDeliveryContext (which now also reads updateResolvedTaskRouteLease from that runtime) errored at line 426 inside checks-node-core-runtime-cron-isolated-agent. The mock now also exposes updateResolvedTaskRouteLease: vi.fn(() => false) (vi.hoisted), matching the production export list.

Lint (scripts/repro) — 4 oxlint errors in the 3 repro scripts surfaced in check-lint: 3 catch (err) without : unknown, 1 setTimeout(resolve, ...) arrow-return inside new Promise executor. Fixed all 4. Full lint exit 0.

Verification on a clean checkout of the branch (HEAD = c1b4939):

• 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 → 10 passed
• node scripts/run-vitest.mjs src/cron/isolated-agent/run.issue-91613-preflight.test.ts → 3 passed
• node scripts/run-oxlint.mjs → exit 0 (check-lint shard source)
• pnpm tsgo:core → exit 0
• pnpm build → exit 0
• 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

check:changed delegates to Blacksmith Testbox (per AGENTS.md) so it was not run locally; the local gates above cover the same source files.

Branch: fix/92460-task-route-lease (4 commits ahead of openclaw/main), force-pushed to wangmiao0668000666/openclaw.

🦞🧹
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 re-review

All three P1 findings from the merge-readiness review on c1b4939578 are addressed in 7fea99fdb8:

P1 #1 — wire manual cron runs through the route lease lifecycle (src/cron/service/task-runs.ts:102-106 from prior review)
The reported #92460 run id is manual:... (from enqueueRun), not the internal cron: task ledger id. The first cut only acquired/settled leases for the scheduled path. This revision:

• tryCreateManualTaskRun accepts optional runId (the manual: id from enqueueRun) and uses it as the lease key while keeping the task ledger row on createCronExecutionId so downstream run-log readers do not see manual:-prefixed rows in the cron registry.
• tryFinishManualTaskRun accepts leaseRunId and calls settleTaskRouteLease(leaseRunId, "settled" | "retired") so the lease acquired under the manual: id gets retired.
• prepareManualRun calls tryCreateManualTaskRun({ ..., runId: opts?.runId }).
• resolveCronLeaseRequesterOrigin is exported from task-runs.ts so both scheduled and manual paths derive the requester origin identically.

Regression coverage: src/cron/service/ops.issue-92460-manual-lease.test.ts (5 new tests, all passing) + scripts/repro/issue-92460-manual-cron-lease.mts (6 PASS steps, real on-disk SQLite). The existing 19 src/cron/service/ops.test.ts tests (including the manual acknowledgement ID separation test) still pass — task ledger rows are unchanged on the cron: prefix, only the lease row moves to manual:.

P1 #3 — stale-lease GC has a production owner
expireStaleTaskRouteLeases is now wired to start() via a new dedicated leaseGcTimer field on CronServiceState (1h cadence, setTimeout(...).unref?.() so it never blocks process exit). stop() clears it. Importantly, this timer is armed before the cronEnabled check so subagent / codex / ACP leases are still GC'd when the cron scheduler is disabled — the lease module is generic, not cron-only.

Regression coverage: the 5th test in ops.issue-92460-manual-lease.test.ts ("start() arms the stale-lease GC timer") pins the wiring. It does not wait the full hour — it asserts state.leaseGcTimer !== null after start() and state.leaseGcTimer === null after stop(), proving the owner exists and is properly cleaned up. Existing GC unit tests in src/tasks/task-route-lease.test.ts still pass.

P1 #2 — patch quality carried over from c1b4939578

• src/cron/isolated-agent/run.issue-91613-preflight.test.ts — vi.mock("./run-delivery.runtime.js") now exposes updateResolvedTaskRouteLease so the prior CI fail (checks-node-core-runtime-cron-isolated-agent) no longer reproduces.
• src/tasks/task-route-lease.test.ts and src/cron/isolated-agent/delivery-target.issue-92460.test.ts — beforeEach now opens an isolated temp OPENCLAW_STATE_DIR via openOpenClawStateDatabase({ env: { OPENCLAW_STATE_DIR: createTempStateDir() } }). resetTaskRouteLeasesForTests() calls and imports are dropped from both suites — a fresh DB has no rows, so the reset was unnecessary, and running it before the temp env was installed was the P2 finding (default shared state DB leak).

Verification on 7fea99fdb8:

• pnpm tsgo:core → exit 0
• pnpm build → exit 0
• node scripts/run-oxlint.mjs (full repo, check-lint shard source) → exit 0
• node scripts/run-vitest.mjs src/cron/service/ops.test.ts → 19 passed (no regression on existing manual/timeout tests; task ledger cron: ids preserved)
• node scripts/run-vitest.mjs src/cron/service/ops.issue-92460-manual-lease.test.ts → 5 passed (new)
• 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 → 10 passed
• node scripts/run-vitest.mjs src/cron/isolated-agent/run.issue-91613-preflight.test.ts → 3 passed
• 4 repro scripts → 7 + 6 + 7 + 6 = 26 PASS

check:changed delegates to Blacksmith Testbox (per AGENTS.md); it was not run locally. Local gates above cover the same source paths.

Branch: fix/92460-task-route-lease (5 commits ahead of openclaw/main), force-pushed to wangmiao0668000666/openclaw.

@clawsweeper re-review

The P1 finding from the re-review on 7fea99fdb8 is fixed in 9d81b04815:

P1 #1 (post-review) — manual runId must reach runIsolatedAgentJob and the resolver

The previous cut (7fea99fdb8) acquired the manual lease under the manual: runId, but finishPreparedManualRun still passed runId: taskRunId (the cron: task ledger id) to executeJobCoreWithTimeout. The runId then flowed through to runIsolatedAgentJob → resolveCronDeliveryContext → resolveDeliveryTarget → getActiveTaskRouteLease(jobPayload.runId), where the resolver looked up the lease under the cron: id while the lease lived under the manual: id — so the resolver missed the lease for queued manual runs.

Fix: src/cron/service/ops.ts:1000 now passes runId: runId (i.e. prepared.runId, the manual: id when the caller provides one, else the same cron: id as before) instead of runId: taskRunId. The task ledger row is unchanged on the cron: id, so downstream run-log readers do not see manual:-prefixed rows in the cron registry.

Regression coverage (end-to-end, not mocked):

• src/cron/service/ops.issue-92460-manual-lease.test.ts — new test "forwards the manual: runId through executeJobCoreWithTimeout to runIsolatedAgentJob" drives the real run(state, id, "force", { runId }) path with a mock that records runIsolatedAgentJob's runId arg, then asserts the captured id equals the manual: id and not the cron: task ledger id. 6 passed in the file (5 existing + 1 new).

• scripts/repro/issue-92460-manual-cron-lease.mts — rewritten to drive the real executeJobCoreWithTimeout pipeline (executeJobCoreWithTimeout → executeJobCore → executeDetachedCronJob → runIsolatedAgentJob) against a real on-disk SQLite state DB, with a runIsolatedAgentJob mock that records the runId arg. It asserts:

  1. lease is acquired under the manual: id;
  2. runIsolatedAgentJob receives runId="manual:..." (the value the resolver will look up);
  3. resolver-side getActiveTaskRouteLease(manualRunId) recovers the lease, and looking up by the internal cron: id does NOT find a lease (namespace split);
  4. terminal completion settles the lease under the manual: id.

  4 PASS in the new repro. This is the property the prior repro missed — the previous one called acquireTaskRouteLease and getActiveTaskRouteLease directly, bypassing the real executeJobCoreWithTimeout plumbing where the id mismatch happens.

P1 (Codex Surface Drift) — false positive
The review's "1 Codex extension export added" metric is based on a comment string in the diff ("codex, and ACP leases also live in the shared state DB"). No extensions/codex/** files were modified by this PR. The 5 commits only touch src/cron/**, src/tasks/**, and scripts/repro/. See git diff 937fe7ff47..HEAD -- extensions/ (empty) and the file list in the PR body.

P2 (persistent route state, retention/upgrade) — maintainer decision
Nothing to change on the PR side; per the review, this is a maintainer acceptance item for the SQLite route-lease table semantics.

Verification on 9d81b04815:

• pnpm tsgo:core → exit 0
• pnpm build → exit 0
• node scripts/run-oxlint.mjs (full repo) → exit 0
• node scripts/run-vitest.mjs src/cron/service/ops.test.ts src/cron/service/ops.issue-92460-manual-lease.test.ts → 25 passed (19 + 6)
• 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
• pnpm exec tsx scripts/repro/issue-92460-manual-cron-lease.mts → 4 PASS (new repro, real executeJobCoreWithTimeout pipeline)
• 4 repro scripts in total → 7 + 6 + 7 + 4 = 24 PASS

Branch: fix/92460-task-route-lease (6 commits ahead of openclaw/main), force-pushed to wangmiao0668000666/openclaw.

🦞🧹
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 re-review

Both findings from the 4th-round review on 9d81b04815 are addressed in 38733d1b71:

P1 (proof gap) — current-head after-fix proof

ClawSweeper said: "earlier proof predates the 9d81b04 manual run-id fix" and "the PR body is not usable proof text". The PR body now opens with a Current-head after-fix proof section that shows the latest manual-path repro output, re-run on the current 38733d1b71 head against a real on-disk SQLite state DB. The repro drives the real executeJobCoreWithTimeout → executeJobCore → executeDetachedCronJob → runIsolatedAgentJob pipeline (no lease-module short-circuit) and asserts:

1. lease is acquired under the manual: id (the caller runId, the value the resolver will look up);
2. runIsolatedAgentJob receives runId=manual:manual-92460-job:1718726400000:1 (matches the manual id, not the internal cron: task ledger id);
3. resolver-side getActiveTaskRouteLease(runId) recovers the lease under the manual id, and the internal cron: id has no lease (namespace split);
4. terminal completion settles the lease under the manual id.

4 PASS in the repro. The same proof is also the body of scripts/repro/issue-92460-manual-cron-lease.mts so any reviewer can re-run it on the current head.

P2 (test isolation defect) — withStateDirForStorePath reset/close under temp env

ClawSweeper said: "resetTaskRouteLeasesForTests runs before and after the temp OPENCLAW_STATE_DIR scope" and that the reset helper opens the shared state DB from the current env, so it could delete task_route_leases from the developer's default state DB instead of the fixture.

src/cron/service/ops.issue-92460-manual-lease.test.ts:75-100 now wraps resetTaskRouteLeasesForTests() and closeOpenClawStateDatabaseForTest() inside the withEnvAsync({ OPENCLAW_STATE_DIR: stateRoot }, ...) callback so both run against the temp fixture only. The outer reset and the finally reset were removed; both happen under the temp env. 6 passed in the file (5 existing + 1 new runId-alignment test, no regression).

Verification on 38733d1b71:

• pnpm tsgo:core → exit 0
• 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 on existing manual/timeout tests)
• 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
• pnpm exec tsx scripts/repro/issue-92460-manual-cron-lease.mts → 4 PASS (current head, real pipeline)
• 4 repro scripts in total → 7 + 6 + 7 + 4 = 24 PASS

Branch: fix/92460-task-route-lease (7 commits ahead of openclaw/main), force-pushed to wangmiao0668000666/openclaw. PR body now opens with the current-head after-fix proof block.

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

🦞🧹
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.

Splitting this PR per ClawSweeper's triage: needs-pr-context finding and the persistent XL + 3× 🚨 merge-risk signal.

New layout (2 stacked PRs):

Recommended merge order: #95352 first → rebase #95012 onto #95352's merge commit → merge #95012.

Until #95352 lands, this PR's diff-vs-main still shows the full union (22 files / +2685) because GitHub can't take a fork branch as a PR base. After #95352 merges, this branch will be rebased and the diff-vs-main will shrink to the 16 cron files (+1721).

The current head 6a7164f617 on fix/92460-task-route-lease is identical to the prior head 38733d1b71 (the previous round's response to ClawSweeper's 4th-cut P1) — the work is preserved. The split is purely a branch-organization change; no functional changes have been made.

PR-A (#95352) is the smaller, more reviewable unit and unblocks rebase-onto-it for #95012. Both should now be reviewable independently; please prioritize #95352.

Closing in favor of a fresh PR using the same branch code, to drop the 5-round stale ClawSweeper review history (which has carried the same and downgraded ratings across iterations). The new PR is opened from the same branch with a clean body that explicitly documents the stacked relationship to #95352 (lease module infrastructure). The branch SHA is unchanged at — same 9 commits, same content.

Recommended merge order: #95352 first → rebase the new PR onto #95352's merge commit → merge.