* fix(cron): default missing sessionTarget on load and guard assertSupportedJobSpec
* fix(cron): use Object.hasOwn for payload.kind check and log the backfill
Address review feedback on #70367:
- Switch the new payload.kind lookup from `in` to `Object.hasOwn` so
prototype pollution cannot drive the defaulter (Aisle Low finding).
- Log a warning when a job is auto-defaulted at load time, matching the
adjacent legacyJobIdIssue pattern so operators can run `openclaw
doctor --fix` to persist the canonical shape (Greptile P2).
* fix(cron): dedupe sessionTarget backfill warn per jobId and sharpen crash site reference
Address deep-review feedback on #70367:
- The code comment referenced assertSupportedJobSpec as the tick-time
crash site, but that function is only called from create/patch
(jobs.ts:607, 686) and manual-run preflight (ops.ts:516). The actual
on-tick TypeError surfaces in runIsolatedAgentJob (server-cron.ts).
Update the comment to say so.
- ensureLoaded runs with forceReload:true on every onTimer tick (~60s).
Before this change, a persistent legacy job missing sessionTarget
produced one warn line per tick, forever. Add a per-jobId dedupe set
on CronServiceState (mirroring the existing warnedDisabled flag) so
the warn fires once per job per process.
- Drop the 'run openclaw doctor --fix' remediation from the warn
message. Doctor's cron-store migration has no trackIssue entry for
missing sessionTarget (doctor-cron-store-migration.ts CronStoreIssueKey),
so doctor --fix on a store whose only defect is missing sessionTarget
silently returns without writing anything. Point operators at
jobs.json directly until that gap is closed.
* docs(changelog): note cron session target repair
---------
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
Addresses codex P1 review on PR #69940: the previous guard rejected
targets that simply omitted accountId, but message-tool fills accountId
from the agent's bound account at exec time (message-tool.ts:730-733),
so account-bound cron jobs legitimately start with target.accountId
undefined. Rejecting that case lost skipMessagingToolDelivery, causing
dispatchCronDelivery to double-send.
Now we only reject when the tool explicitly names a *different*
accountId — which is the real CWE-284 spoof vector. Omission matches.
Tests updated accordingly:
- matcher unit test: flipped "omit accountId" case from false to true;
"accountIds differ" case preserved as the real spoof guard
- integration tests: one legitimate-default case (rewrite happens),
one explicit-mismatch case (rewrite suppressed)
658 cron tests pass.
When a cron job sends via the generic `message` tool, the delivery trace
previously recorded `messageToolSentTo[i].channel = "message"` even
though the send was resolved to a specific channel (e.g. telegram). This
made `jq` diffing intended-vs-actual awkward for the happy path.
Fix:
- `normalizeMessagingToolTarget` now rewrites `channel: "message"`
to the resolved channel when `matchesMessagingToolDeliveryTarget`
confirms the tool send matches the resolved cron delivery target.
Genuinely unmatched generic sends keep the literal "message" so
audits can still flag them.
- `matchesMessagingToolDeliveryTarget` now requires strict accountId
equality whenever the resolved delivery carries an `accountId`. An
omitted `target.accountId` previously short-circuited the guard and
was treated as a wildcard, letting a generic send spoof attribution to
any bot identity in the cron delivery trace (CWE-284). This was
flagged by Aisle on #69771.
Tests:
- Unit: `matchesMessagingToolDeliveryTarget` rejects omitted-accountId
against account-tied delivery; still matches same-accountId.
- Integration: cron run trace rewrites generic "message" to the
resolved channel, preserves accountId on both sides, and leaves the
literal "message" provider in place when the tool send omits
accountId against an account-tied delivery.
The bug: three persist sites accumulated cost instead of snapshotting
it like tokens. This caused cost to be inflated 1x-72x on multi-persist
sessions because the same cumulative usage was added repeatedly.
Root cause: persistSessionUsageUpdate, updateSessionStoreAfterAgentRun,
and the cron isolated-agent run path all used:
estimatedCostUsd = existingCost + runCost
But runCost was already computed from cumulative run usage, so this
added the same cost repeatedly on redundant persists.
Fix: snapshot cost directly like tokens already do:
estimatedCostUsd = runCost
Files affected:
- src/auto-reply/reply/session-usage.ts
- src/agents/command/session-store.ts
- src/cron/isolated-agent/run.ts
Tests added:
- session-store.test.ts: verify cost is snapshotted, not accumulated
- session.test.ts: updated existing test to verify snapshot behavior
Fixes#69347
