fix(skills): refresh stale skill snapshot after gateway restart#12209
fix(skills): refresh stale skill snapshot after gateway restart#12209mcaxtr wants to merge 5 commits intoopenclaw:mainfrom
Conversation
Additional Comments (1)
Consider computing Prompt To Fix With AIThis is a comment left during a code review.
Path: src/auto-reply/reply/session-updates.ts
Line: 186:188
Comment:
**Missed refresh on first turn**
`shouldRefreshSnapshot` is computed from `nextEntry` (initially `sessionEntry`), but in the `isFirstTurnInSession` block you may actually use `current = sessionStore[sessionKey]` when `sessionEntry` is undefined. In that case `persistedVersion` becomes `0` even if `sessionStore` already has a `skillsSnapshot.version > 0` from a prior process, so the restart refresh case (`snapshotVersion === 0 && persistedVersion > 0`) won’t trigger and the stale snapshot can be reused on the first turn.
Consider computing `persistedVersion` from `current` (the entry you actually read from) or recomputing `shouldRefreshSnapshot` inside the first-turn branch after `current` is resolved.
How can I resolve this? If you propose a fix, please make it concise. |
|
Addressed in c5fc94c — good catch on the semantic inconsistency.
In practice, the behavior was already correct for all paths:
But the fix makes the code robust against future refactors that might remove those independent guards. Added a dedicated test for the edge case (test 5 in the suite). |
|
@greptile review |
1 similar comment
|
@greptile review |
|
@greptile review |
Fixes openclaw#13377 Sessions snapshot allowAgents permissions at creation time and never refresh them, even after a full gateway restart. This adds version- tracked allowAgents snapshots with dual-condition restart detection (mirroring the skills snapshot pattern from PR openclaw#12209) so that existing sessions pick up the current config on their next turn.
16b81b4 to
40c2467
Compare
40c2467 to
02adee9
Compare
02adee9 to
4989f4a
Compare
e6ddb34 to
2f52958
Compare
bfc1ccb to
f92900f
Compare
b2ec290 to
f562f13
Compare
After a gateway restart, the in-memory skills version resets to 0 while existing sessions retain snapshots with version > 0 from the prior process. The shouldRefreshSnapshot check required snapshotVersion > 0, so it never triggered a rebuild — leaving sessions with permanently stale skill lists. Detect the restart scenario (in-memory version 0, persisted version > 0) and rebuild the snapshot on the next message. Fixes openclaw#12092
…ntry is undefined
f562f13 to
d3d0a50
Compare
Summary
shouldRefreshSnapshotcheck requiredsnapshotVersion > 0, so it never triggered a rebuild after restartRoot Cause
getSkillsSnapshotVersion()returns from in-memory state (workspaceVersions/globalVersioninrefresh.ts), which resets to 0 on process restart. The comparison atsession-updates.ts:147-148was:Since
snapshotVersionis 0 after restart, the conditionsnapshotVersion > 0is always false, so stale snapshots are reused forever.Fix
Extend the condition to also detect the restart scenario:
The second clause detects: "process just started (version 0) but session has a snapshot from a prior lifetime (version > 0)" → rebuild.
Test Plan
session-updates.ts)pnpm buildpassespnpm checkpasses (lint + format)codex review --base mainreturns zero issuesTDD: All 4 new tests fail before, pass after
Fixes #12092
Greptile Overview
Greptile Summary
This PR updates
ensureSkillSnapshotto correctly refresh a session’s persisted skills snapshot after a gateway restart. It introduces a restart-detection clause: if the in-memory snapshot version resets to0(fresh process) but the session already has a persisted snapshot withversion > 0from a prior process lifetime, the snapshot is rebuilt instead of reused indefinitely.It also adds a focused Vitest suite covering:
sessionEntryis missing butsessionStorecontains the stale snapshotThese changes fit into the existing skills refresh model where
getSkillsSnapshotVersion()is maintained in-memory (and resets on restart), while session snapshots are persisted in the session store.Confidence Score: 5/5
ensureSkillSnapshot, and the new logic deterministically rebuilds only when the persisted snapshot is known-stale relative to the process lifetime or version bump.