feat: keep reset session history searchable#111194
Conversation
openclaw-barnacle
Bot
added
docs
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs real behavior proof before merge. Reviewed July 19, 2026, 6:33 AM ET / 10:33 UTC. Summary PR surface: Source +454, Tests +559, Docs +2. Total +1015 across 27 files. Reproducibility: not applicable. This PR proposes a new reset-history retention model rather than reporting a standalone current-main bug. Its intended behavior is nevertheless source-testable through reset, search, and physical-budget scenarios. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Preserve an explicit operator-controlled retention contract for reset-generated SQLite history, move physical-budget scheduling to a shared post-transcript-commit seam, and prove reset search plus disk-pressure reclamation with redacted live output before merge. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR proposes a new reset-history retention model rather than reporting a standalone current-main bug. Its intended behavior is nevertheless source-testable through reset, search, and physical-budget scenarios. Is this the best way to solve the issue? No, not as written. A common post-transcript-commit enforcement seam and an owner-approved retention contract for existing finite settings are safer than the current asynchronous-hook and disk-only approach. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against b9c9140034d7. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +454, Tests +559, Docs +2. Total +1015 across 27 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
Review history (3 earlier review cycles)
|
c43fddc to
7e847b8
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c43fddcffc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const files = (await readSessionsDirFiles(path.dirname(params.storePath))) | ||
| .filter((file) => isRetainedSessionTranscriptArchiveName(file.name)) | ||
| .toSorted((left, right) => left.mtimeMs - right.mtimeMs); |
There was a problem hiding this comment.
When an upgraded store contains legacy *.jsonl.bak.* or sessions.json.bak.* archives, physical measurement counts those files, but this filter only permits reset and deleted archives to be removed. Enforcement can therefore evict searchable SQLite generations while leaving the older backup artifacts untouched, and may still finish above highWaterBytes; include every removable archive recognized by isSessionArchiveArtifactName, or exclude unsupported archive types from the measured total.
Useful? React with 👍 / 👎.
| kickSessionHistoryDiskBudgetMaintenance({ | ||
| ...(resolved.agentId ? { agentId: resolved.agentId } : {}), | ||
| storePath: resolveSessionStorePathForScope(scope), | ||
| ...(options.maintenanceConfig ? { maintenanceConfig: options.maintenanceConfig } : {}), | ||
| }); |
There was a problem hiding this comment.
When transcript growth is committed through a synchronous writer, this async-entry-only hook never runs; for example, gateway/worker-environments/transcript-commit.ts:381 appends transcript data and calls replaceSessionEntrySync, which has no budget kick. If retained history is initially below the cap and worker commits later push the database or WAL over maxDiskBytes, it remains over budget until an unrelated reset, delete, async patch, or manual cleanup occurs, so the throttled check needs to live in a common transcript/post-commit seam or cover the sync writer too.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
7e847b8 to
340da9f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 340da9fbd9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
| const historicalArchivedTranscripts: SessionLifecycleArchivedTranscript[] = []; | ||
| for (const sessionId of historicalGenerationIds) { |
There was a problem hiding this comment.
When a guarded delete arrives with a stale expectedEntry, expectedSessionId, lifecycle revision, or timestamp, this loop archives and commits deletion of every retained generation before shouldDeleteSqliteSessionEntryLifecycle evaluates those guards in the final transaction. The call then returns deleted: false and preserves the live entry, but its reset history has already been removed from SQLite; validate the current entry against all requested guards before starting historical reclamation.
Useful? React with 👍 / 👎.
| export async function measureSessionPhysicalDiskUsage( | ||
| storePath: string, | ||
| ): Promise<SessionPhysicalDiskUsage> { | ||
| const sessionsDirFiles = await readSessionsDirFiles(path.dirname(storePath)); |
There was a problem hiding this comment.
When supported custom session.store targets share a directory, such as {agentId}.json, this directory-wide scan counts sibling SQLite/WAL files and unrelated files toward the current store's budget; archive pruning later scans the same shared directory. A large sibling store can therefore trigger enforcement for a small store, prune archives belonging to either store, and evict all history from the small store without relieving the actual source of pressure. Count and prune only artifacts owned by the resolved SQLite target/store.
Useful? React with 👍 / 👎.
340da9f to
a4d21b6
Compare
a4d21b6 to
a7abdc7
Compare
clawsweeper
Bot
added
rating: 🦪 silver shellfish
a7abdc7 to
b033563
Compare
b033563 to
568ce44
Compare
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Merged via squash.
|
* feat(sessions): retain reset history in sqlite with physical disk budget * fix(sessions): satisfy test-types, knip export scan, and docs map * fix(sessions): align behavioral suites and flip proof with retained history, route-aware cleanup plans
1 participant
What Problem This Solves
/resetand/newpreviously destroyed the old conversation: rows were archived to a compressed file and deleted from SQLite, so past sessions disappeared from search and history. For a personal agent, sessions should be forever — the natural boundary is compaction, and old history should only be reclaimed under explicit deletion or disk pressure.Why This Change Was Made
Follows the Codex philosophy (no scheduled resets; compaction is the boundary; extract/compress under pressure, never silently discard) adopted for OpenClaw's personal-agent architecture, building on #111140 (no automatic session resets by default). No DDL, no schema-version change, no new config: existing
session.maintenancekeys (maxDiskBytes10gb default,highWaterBytes80%) bound retained history physically.User Impact
sessionKey -> sessionIdmapping advances; previous generations' session, transcript, trajectory, and search rows stay in SQLite and remain searchable under the same key. Entry/session lists show only the live mapping.maxDiskBytes, enforcement checkpoints, prunes oldest archives, then extracts-and-evicts oldest unreferenced historical generations tohighWaterBytes. Passes are serialized per store; entry writes/reset/delete kick a throttled (reset/delete: forced) background pass.Consciously accepted tradeoffs (in-code comments pin each):
resetArchiveRetentioncontinues to age archive artifacts only. Age-based expiry of retained rows needs a durable retirement timestamp (additivesessionscolumn) — deliberately excluded; follow-up needs a schema decision.Evidence
session-history-eviction,store.session-lifecycle-mutation,store.pruning(.integration),disk-budget,lifecyclesuites;pnpm tsgo:coreclean;git diff --checkclean.wouldMutate, checkpoint-delta reporting, budget-priority archive pruning, forced kicks after reset/delete with pending-force coalescing, migration-archive exclusion regression test.docs/concepts/main-session.md,docs/reference/session-management-compaction.md.