fix(sessions): prune stale gateway model-run sessions - #91057
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 24, 2026, 4:35 PM ET / 20:35 UTC. Summary PR surface: Source +200, Tests +365, Docs +16. Total +581 across 19 files. Reproducibility: yes. Source inspection shows current main creates agent-scoped explicit model-run session rows while current and latest-release maintenance only apply global prune/cap retention, and the linked issue provides production counts showing those rows saturating the cap. Review metrics: 3 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land this or an equivalent core session-lifecycle fix after maintainers explicitly accept pressure-gated cleanup of stale strict model-run probe rows and normal merge gates pass. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main creates agent-scoped explicit model-run session rows while current and latest-release maintenance only apply global prune/cap retention, and the linked issue provides production counts showing those rows saturating the cap. Is this the best way to solve the issue? Yes, pending maintainer acceptance. Session maintenance is the right owner because it repairs already-polluted stores, keeps matching strict to generated model-run UUID keys, avoids a new public config option, and preserves old-shape plugin maintenance-config callers. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d23977edbcf9. Label changesLabel justifications:
Evidence reviewedPR surface: Source +200, Tests +365, Docs +16. Total +581 across 19 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Updated the PR body with structured real-behavior proof fields (, , , , , ). I also validated the body locally against ; it now passes the proof policy for the current head. |
|
Updated the PR body with structured real-behavior proof fields: behavior, environment, steps, evidence, observedResult, and notTested. I also validated the body locally against scripts/github/real-behavior-proof-policy.mjs; evaluateRealBehaviorProof now passes for the current head. |
8413871 to
fc22cc1
Compare
|
Rebased onto latest upstream/main and updated the structured real-behavior proof for the new head. Local checks after rebase:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Addressed the remaining ClawSweeper docs finding by documenting session.maintenance.modelRunPruneAfter in public session maintenance and cleanup docs. Docs checks:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@jalehman — apologies for the direct ping; this builds on the session-maintenance transaction seam you added in #93737. ClawSweeper currently has it as As far as I can tell, the remaining question is the product/default-behavior call: stale strict Happy to switch this to default-off if you'd prefer. No rush. |
All good! This is something I'd like to be pinged about. Will review and incorporate today. |
The model-run prune predicate fell back to testing the raw sessionKey when parseAgentSessionKey returned null, so unscoped keys like `explicit:model-run-<uuid>` and shapes with empty agent ids were eligible for the new default 24h cleanup. Restrict matching to keys that successfully parse as agent-scoped with a non-empty agent id, and add negative tests covering unscoped, empty-agent, extra-segment, and whitespace-padded keys. Refs openclaw#88632 (review feedback before merge).
Forced maintenance (sessions cleanup / maintenanceOverride) caps immediately to maxEntries, but the unset model-run default was high-water gated. In the (maxEntries, high-water) window stale model-run probes survived while the forced cap evicted real sessions — the inverse of openclaw#88632. shouldRunModelRunPrune now takes a force flag: when the caller caps immediately, the unset default prunes once entryCount > maxEntries. Wire force at the two forced call sites (applyEnforcedMaintenance, previewStoreCleanup). Make the SDK runtime config field modelRunPruneAfterConfigured optional (additive). Add force-gate unit test + forced-apply regression test.
The model-run maintenance fields (modelRunPruneAfterMs from openclaw#88632 base work, modelRunPruneAfterConfigured from the pressure-gating fix) were required on the resolved maintenance config exposed to plugins via patchSessionEntry's maintenanceConfig. External plugin TypeScript callers that construct a pre-openclaw#88632 maintenanceConfig would fail to compile. Make both fields optional on ResolvedSessionMaintenanceConfig (and the runtime type), so old-shape plugin configs keep compiling. All internal readers already treat an absent value as unset: shouldRunModelRunPrune returns false when modelRunPruneAfterMs == null and modelRunPruneAfterConfigured is falsy, so a plugin-supplied config without the fields runs no model-run pruning — the pre-openclaw#88632 behavior. The resolver still always populates both fields, so normal runtime behavior is unchanged. Add an old-shape maintenanceConfig SDK regression test.
Dependency graph guard clearedThis PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh
|
|
Merged via rebase.
|
Summary
Prunes stale gateway model-run probe sessions (
agent:*:explicit:model-run-<uuid>) before they can contribute to session cap pressure and evict durable sessions (#88632).The cleanup is intentionally not exposed as a new
openclaw.jsonoption. OpenClaw applies a fixed 24h model-run probe retention only when session-entry maintenance/cap pressure is reached. Healthy stores are left untouched, and strict key matching keeps normal direct, group, thread, cron, hook, heartbeat, ACP, and sub-agent sessions out of this cleanup path.modelRunPruned/prune-model-runare reported separately in cleanup summaries and dry-run plans.Fixes #88632
Real behavior proof
Behavior addressed: stale gateway
model-run-<uuid>probe sessions are pruned before themaxEntriescap can evict durable sessions, while healthy stores below cap pressure are left untouched.Real environment tested: local macOS worktree at head
5d9a14ec360(josh/review-pr-91057-thermonuclear), using the realpnpm openclaw sessions cleanupCLI after the worktree runtime build, with isolatedOPENCLAW_CONFIG_PATHvalues and temp session stores under/tmp/openclaw-91057-proof.LB3erh.Exact steps or command run after this patch:
Evidence after fix: the fixture stores contained one stale strict model-run probe (
agent:main:explicit:model-run-123e4567-e89b-12d3-a456-426614174100, updated 2 days ago), one recent strict model-run probe (...174101, updated now), and one durable thread session (agent:main:discord:channel:123456:thread:987654, updated 2 days ago).No-pressure dry-run result:
{ "beforeCount": 3, "afterCount": 3, "modelRunPruned": 0, "capped": 0, "wouldMutate": false, "staleStillPresent": true }Pressure dry-run result:
{ "beforeCount": 3, "afterCount": 2, "modelRunPruned": 1, "capped": 0, "wouldMutate": true }Text dry-run showed the visible cleanup action:
Apply result:
{ "beforeCount": 3, "afterCount": 2, "modelRunPruned": 1, "capped": 0, "applied": true, "staleRemoved": true, "recentModelRunPresent": true, "durableThreadPresent": true }Removed-config proof:
Observed result after fix: model-run cleanup is pressure-gated under the fixed 24h policy, removes the stale probe before capping, reports
modelRunPruned/prune-model-run, preserves recent model-run and durable thread sessions, and rejects the removed publicsession.maintenance.modelRunPruneAfterkey.What was not tested: no live remote model/provider call was required because this PR changes session-store cleanup behavior, not model execution or transport delivery. The future SQLite storage flip was not tested here; this proof exercises the current file-backed store and real cleanup CLI path.
Testing
node scripts/run-vitest.mjs src/plugin-sdk/session-store-runtime.test.ts src/config/sessions/store.pruning.test.ts src/config/sessions/store.pruning.integration.test.ts src/commands/sessions-cleanup.test.tspnpm tsgo:core:test.agents/skills/autoreview/scripts/autoreview --mode branch --base aa0bdb901f4a166bd9652b5b4e397febd3fce91fgit diff --check HEADCurrent status (2026-06-24)
Maintainer follow-up removed the proposed public config option, closed the internal resolved maintenance type back up, and restored plugin-facing source compatibility for old-shape
maintenanceConfigcallers via a normalized input type. The PR now implements only the automatic fixed-policy cleanup and the CLI/reporting proof needed for #88632.maintainerCanModifyis enabled, and the PR branch has been updated to head5d9a14ec360.