openclaw - 💡(How to fix) Fix Stale-entry writeback races for lastChannel/channel in agent handler session patch [1 participants]

Context

PR #19328 (fix for #5369) removed most stale-entry writebacks from the session-entry patch in src/gateway/server-methods/agent.ts. Two residual patterns in the same function still carry a stale cached value into the patch and can clobber a fresh concurrent write via the {...freshStore, ...patch} merge in updateSessionStore.

Residual patterns

At roughly src/gateway/server-methods/agent.ts:675-681:

lastChannel: effectiveDeliveryFields.lastChannel ?? entry?.lastChannel,
lastTo: effectiveDeliveryFields.lastTo ?? entry?.lastTo,
lastAccountId: effectiveDeliveryFields.lastAccountId ?? entry?.lastAccountId,
lastThreadId: effectiveDeliveryFields.lastThreadId ?? entry?.lastThreadId,
// ...
channel: entry?.channel ?? request.channel?.trim(),

Both patterns write entry?.X (stale cached read) into the patch. Failure mode is the same shape as #5369:

1. loadSessionEntry returns cached entry (stale; missing the field)
2. A concurrent write (e.g. sessions.patch) updates the field in the store
3. The agent handler's updateSessionStore mutator merges {...freshStore, ...nextEntryPatch}
4. nextEntryPatch.X = undefined (stale) clobbers the fresh store value

Why they weren't fixed in #19328

Narrower blast radius than #5369:

• lastChannel/lastTo/etc. only race when both effectiveDeliveryFields.X and entry?.X are undefined but the fresh store has a value — an unusual combination.
• channel rarely changes mid-session, so stale cache typically matches fresh.

Neither was part of the reported user impact (sub-agent model override failures), so they were held out of the minimal fix to keep the PR focused and reviewable.

Proposed fix

Same pattern as #19328: drop the stale-cache writebacks from the patch and let mergeSessionEntry's {...existing, ...patch} spread preserve fresh store values. For the delivery fields, this means either conditional property inclusion (only set when effectiveDeliveryFields.X is defined) or moving the delivery merge into the updateSessionStore mutator so it reads fresh lastChannel as the fallback.

Rough sketch:

const patch: Partial<SessionEntry> = {
  sessionId,
  updatedAt: now,
  deliveryContext: effectiveDeliveryFields.deliveryContext,
  label: labelValue,
  spawnedBy: spawnedByValue,
  groupId: resolvedGroupId ?? undefined,
  groupChannel: resolvedGroupChannel ?? undefined,
  space: resolvedGroupSpace ?? undefined,
};
if (effectiveDeliveryFields.lastChannel !== undefined) patch.lastChannel = effectiveDeliveryFields.lastChannel;
if (effectiveDeliveryFields.lastTo !== undefined) patch.lastTo = effectiveDeliveryFields.lastTo;
// ...etc
if (request.channel?.trim() && !entry?.channel) patch.channel = request.channel.trim();

Regression test: extend the "stale cached entry / fresh store" pattern established by the #5369 broader test in agent.test.ts to cover these fields.

References

• #5369 — the modelOverride race that prompted the investigation
• #19328 — the minimal fix that established the "no stale writebacks in patch" pattern
• agent.test.ts tests preserves fresh modelOverride when cached entry is stale (#5369) and preserves all fresh session fields when cached entry is stale (#5369 broader) — template for the regression tests here