Repository navigation
fix(cron): preserve current-session delivery intent - #139491
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. Reviewed September 5, 2026, 10:06 PM ET / September 6, 2026, 02:06 UTC. ClawSweeper reviewWhat this changesPreserves current-session automation delivery intent across routing, previews, and completion reporting, with regression coverage and a native process-probe fixture correction. Merge readiness✅ Ready for maintainer review The fix remains necessary: current main and the latest release retain the affected fallback. No actionable introduced defect remains, and the previous suppression-inventory finding is resolved. Priority: P2 Review scores
Verification
How this fits togetherScheduled agent turns produce reports for their bound conversations. Cron delivery combines requested destinations with session routing, commits the report, and records whether an external send is still owed. flowchart TD
A[Scheduled agent turn] --> B[Prepared delivery intent]
C[Conversation route] --> D[Resolve destination]
B --> D
D --> E[Commit conversation report]
E --> F{External delivery owed?}
F -->|No| G[Delivered]
F -->|Yes| H[Send or record delivery error]
Before mergeNone. Agent review detailsSecurityNone. PR surfaceSource -3, Tests +219, Docs +2. Total +218 across 15 files. View PR surface stats
Review metricsNone. Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Technical reviewBest possible solution: Keep one prepared delivery-intent decision shared by preview and completion, preserving conversation reports and explicit destination failures. Do we have a high-confidence way to reproduce the issue? Yes: current source supports the documented dashboard reproduction with one unrelated configured channel. This review did not execute it; the supplied real Gateway evidence records before/after outcomes. Is this the best way to solve the issue? Yes: carrying existing session intent into the canonical resolver addresses the cause without adding configuration, storage formats, or a competing delivery path. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against e915ac8ec5be. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (5 earlier review cycles)
|
e8a77b8 to
a3bef6a
Compare
|
@clawsweeper re-review The final repair at a3bef6a now preserves both sides of current-session delivery intent. An unrelated configured channel does not create an external obligation; explicit recipient/account/thread coordinates retain their delivery error when channel selection fails. Preview and completion share the prepared-plan decision, and the conversation report remains committed in both cases. All 204 focused tests, canonical core/services type checks, runtime build, and fresh managed P0–P2 review passed. Two refreshed real Gateway/dashboard flows on this exact clean head prove Delivered for local-only intent and Not delivered for an unresolved explicit recipient, with reports retained and zero external sends. Inspected synthetic recordings are in the updated PR body. Production delta is -3 lines. The CI fixture now uses the existing activated outbound adapter; no timeout or assertion was weakened. The rebase also inherits the unrelated upstream CI assertion correction in #139519. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Carry the normalized delivery plan through CLI suppression and Gateway background-completion fixtures. Preserve their assertions and the existing runtime requirement. Record the existing native drain capture suppression introduced by #139483 in the intentional inventory, without changing its receiver semantics.
Upstream #139576 now observes the native drain without an unbound-method suppression. Remove the obsolete inventory entry so the combined CI tree matches current main.
Default the shared spawnSync mock to Node so Linux cleanup probes remain observable. Preserve the explicit report-unit overrides and strict process-tree checks.
|
Merged via squash.
|
* commit '6c55a99226ed0611ca088004c81b00fb296f1f71': (182 commits) test(ui): scope dashboard settings setup to the top frame (openclaw#139618) refactor(skills): share pending proposal creation (openclaw#139617) fix(release): accept frozen targets before worker bundles (openclaw#136699) fix(gateway): preserve percent sequences in media filenames (openclaw#139508) fix: preserve current update availability alongside update history (openclaw#139501) feat(diagnostics): expose garbage collection duration metrics (openclaw#139592) fix(ui): keep Logbook timestamps clear of the activity stripe (openclaw#139539) fix(ui): keep Swarm outcomes visible after completion (openclaw#139490) test: check file accessibility without runtime-specific return values (openclaw#139590) refactor(sdk): keep helper types private (openclaw#139601) fix(cron): preserve current-session delivery intent (openclaw#139491) fix: prevent browser profile permission hangs in Doctor (openclaw#139612) fix(agents): repair storage-failure test fixture after failover owner move (openclaw#139531) fix(cli): keep note text with Windows and Unicode line endings (openclaw#139512) fix(agents): keep isolated completion context on its admitted generation (openclaw#139607) fix: avoid declaration builds during source-server updates (openclaw#139392) fix(models): keep discovered models after catalog refresh failures (openclaw#139357) fix(gateway): record a transcript notice when a run fails before replying (openclaw#139439) refactor(ios): reuse shared integer conversion (openclaw#139581) fix(sessions): prevent database lock failures during reclamation (openclaw#139469) ...
* commit 'refs/openclaw/tasks/pr-135167-main-latest': test(ui): scope dashboard settings setup to the top frame (#139618) refactor(skills): share pending proposal creation (#139617) fix(release): accept frozen targets before worker bundles (#136699) fix(gateway): preserve percent sequences in media filenames (#139508) fix: preserve current update availability alongside update history (#139501) feat(diagnostics): expose garbage collection duration metrics (#139592) fix(ui): keep Logbook timestamps clear of the activity stripe (#139539) fix(ui): keep Swarm outcomes visible after completion (#139490) test: check file accessibility without runtime-specific return values (#139590) refactor(sdk): keep helper types private (#139601) fix(cron): preserve current-session delivery intent (#139491) fix: prevent browser profile permission hangs in Doctor (#139612) fix(agents): repair storage-failure test fixture after failover owner move (#139531) fix(cli): keep note text with Windows and Unicode line endings (#139512) fix(agents): keep isolated completion context on its admitted generation (#139607) fix: avoid declaration builds during source-server updates (#139392) fix(models): keep discovered models after catalog refresh failures (#139357) fix(gateway): record a transcript notice when a run fails before replying (#139439)
* fix(cron): keep dashboard current-session delivery in its conversation * fix(cron): retain unresolved explicit delivery failures * test: complete cron delivery fixtures and CI inventory Carry the normalized delivery plan through CLI suppression and Gateway background-completion fixtures. Preserve their assertions and the existing runtime requirement. Record the existing native drain capture suppression introduced by openclaw#139483 in the intentional inventory, without changing its receiver semantics. * test: drop superseded drain suppression entry Upstream openclaw#139576 now observes the native drain without an unbound-method suppression. Remove the obsolete inventory entry so the combined CI tree matches current main. * test: keep native process probes in report fixture Default the shared spawnSync mock to Node so Linux cleanup probes remain observable. Preserve the explicit report-unit overrides and strict process-tree checks.
Closes #139488
What Problem This Solves
Fixes incorrect delivery outcomes for current-session agent-turn automations. A dashboard/WebChat report could be marked Not delivered merely because an unrelated external channel was configured. Conversely, an explicitly requested destination could be silently treated as local delivery when no channel could be selected.
Why This Change Was Made
The delivery resolver now receives the job's existing current-session intent, so it does not invent an external route from unrelated host configuration. A shared delivery-intent check combines the normalized plan with the resolved source route: an explicit or remembered external destination remains owed even when resolution fails. Preview and completion use the same check, and completion receives the plan prepared before inference rather than re-reading job configuration afterward.
The conversation report is committed in both cases. A local-only report completes delivery; an unresolved explicit external request retains its delivery error. Command announcements keep their strict external path. No configuration, schema, protocol, or stored representation changes are introduced. Production code shrinks by three lines.
User Impact
Current-session reports finish successfully in their bound conversation when no external delivery was requested. Requested destinations remain visible as delivery failures if channel selection cannot resolve them. Preview and run history agree about the outcome, while the report remains available in chat.
Evidence
The original real Gateway/dashboard before capture shows the report in chat but a false Telegram missing-target failure with one unrelated configured channel. The repair has storage/resolver coverage for dashboard and WebChat, zero/one/two configured channels, explicit channel/recipient/account/thread settings including thread zero, unavailable actual external routes, and command announcements.
Independent review exposed the explicit-destination side of the same invariant. Nine new cases failed before the correction; the three completion cases verified the durable report before failing on the omitted delivery error. The full final four-file test selection then passed 204 tests. Canonical core/services test types, targeted lint, formatting, and the runtime/metadata build passed.
The first CI run stalled in a recipient test because its metadata-only fake plugin triggered real plugin bootstrap. The fixture now uses the existing activated outbound adapter while retaining the same configured account and assertions. A cold call-through probe observed three bootstrap calls before and zero after; the original 20 cases passed with a fresh cache. No timeout, assertion, or production loader was weakened.
The runtime build uses
sourcePerformance, not full SDK/package output. All external APIs in the real Gateway/browser proof are synthetic and local; no authenticated cloud-provider or real external-message claim is made. The original before proof used packaged-plugin staging; subsequent runtime proof uses supported source staging after an earlier package preflight hit its existing 120-second timeout before Gateway execution.The runtime-validated production commit is
a3bef6a3b2cd1755287155dffa52be96a9758c9e, rebased onto61690da40bd436684311adec59d69bae3b0e383dto inherit the unrelated upstream CI correction in #139519. The complete four-file selection passed 204 tests again on this clean head, as did canonical core/services type checks and the runtime/metadata build. Fresh managed P0–P2 review is scoped-clean. The seven production owners remain unchanged on fetched maine284dad94f9c26cf91f0ce69a392890c7bb6de1e.This completes the no-external-route behavior of #131975 and preserves the opposite explicit-destination obligation. It is separate from creation-time capture of an explicitly selected channel in #138185.
Fresh real Gateway/dashboard flows on that exact built head verified both outcomes: route-less report with one unrelated Telegram channel → report retained, Delivered, completion succeeded; explicit recipient with zero channels → report retained, Not delivered, completion failed with the channel-resolution error. Each made one synthetic provider request and zero external sends. Both drivers verified their actual HOME, state, config, OAuth, XDG and announced log-file paths were inside their owned temporary roots.
attachment-2939456438.mp4
attachment-758333510.mp4
Current head
6288cf4e58df835fbffbd7ee9858b737e6d32900contains test-only CI corrections relative to the runtime-tested production commit above. Production is byte-identical. CI exposed two additional direct dispatcher fixtures in CLI suppression and Gateway background-completion coverage that needed the prepared plan. The complete caller audit covers all five construction owners. Those two files passed 31 tests and their owning type checks; the complete candidate passed managed P0–P2 review.CI also caught an incoming-main lint-suppression inventory omission from #139483. While the temporary inventory correction was being validated, #139576 removed the suppression at its source. The final branch therefore leaves that inventory unchanged; the superseded entry was removed and that deletion independently reviewed. The original three fixture/guard checks totalled 35 passing tests, and current CI verifies composition with the upstream cleanup. No assertions, timeouts, production fallback, or helper behavior were weakened.
A later Linux CI failure occurred after the native JSON-report test had written its report but encountered strict process-group cleanup. Its broad
spawnSyncmock had been reset to returnundefined, also affecting Linux process-inspection probes. The fixture now calls Node's actual implementation by default and retains the explicit report-unit overrides. All seven report-helper tests, root-test types, and selected static/lint checks passed, followed by scoped-clean P0–P2 review. Strict cleanup and its deadlines are unchanged. The historical group member was not recorded; no specific descendant is blamed. Exact-head CI supplies the remaining Linux proof.