refactor: move session metadata to SQLite#91322
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 8, 2026, 1:33 AM ET / 05:33 UTC. Summary Reproducibility: yes. Source inspection shows the latest PR head removes the current-main existence filter and then calls Review metrics: 2 noteworthy metrics.
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
Security Review findings
Review detailsBest possible solution: Land only after the doctor preview path is read-only, a latest-release-style session-store upgrade is proven through startup/doctor/CLI read paths, and maintainers explicitly accept the SDK/storage compatibility change. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows the latest PR head removes the current-main existence filter and then calls Is this the best way to solve the issue? No. The SQLite migration direction is plausible, but this is not the best merge shape until preview scans use an existing-store read-only path and release-style upgrade behavior is proven. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against b8adc11977ab. Label changesLabel justifications:
Evidence reviewedWhat 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
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c119b8b089
ℹ️ 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 targets = resolveAllAgentSessionStoreTargetsSync(params.cfg, { | ||
| env: params.env ?? process.env, | ||
| }).filter((target) => fs.existsSync(target.storePath)); | ||
| }); |
There was a problem hiding this comment.
Avoid creating session DBs during doctor preview
When session.store is an explicit non-agent path that does not exist yet, resolveAllAgentSessionStoreTargetsSync still returns it, and the preview branch below calls loadSessionStore, which now opens/creates the SQLite backing file. That means a read-only openclaw doctor preview for Codex route warnings can leave a new openclaw-session-store-*.sqlite beside the configured store even when there are no sessions to scan; keep the old existence/read-only behavior for preview scans or use the read-only SQLite loader here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 704a521681
ℹ️ 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".
| function resolveSessionStoreDatabaseOptions(storePath: string): OpenClawAgentDatabaseOptions { | ||
| const structuralStateDir = resolveStateDirFromSessionStorePath(storePath); | ||
| const agentId = resolveAgentIdFromSessionStorePath(storePath) ?? DEFAULT_AGENT_ID; | ||
| if (structuralStateDir) { |
There was a problem hiding this comment.
Keep SQLite stores in the discovered agent directory
When a legacy store lives under an agent directory whose basename does not round-trip through normalizeAgentId (for example agents/Retired Agent/sessions/sessions.json), this structural branch resolves the SQLite DB using the normalized id, so opening/migrating it creates a new sibling agents/retired-agent/agent/openclaw-agent.sqlite. The target discovery code scans every directory under agents/ and dedupes by storePath, so after migration both the original Retired Agent/sessions/sessions.json path and the new normalized retired-agent/sessions/sessions.json path resolve to the same DB and can list or clean the same sessions twice; keep the DB path tied to the actual discovered agent directory or dedupe by resolved SQLite path.
Useful? React with 👍 / 👎.
|
Land-ready verification for head Local proof run:
Earlier branch proof retained after the SQLite session-store/cache fixes:
Autoreview:
CI on this head is clean:
Note: |
* refactor: move session metadata to sqlite * test: seed session stores with sqlite fixtures * test: seed remaining session stores with sqlite fixtures * fix: stabilize sqlite session cache freshness * test: seed cli transcript metadata in sqlite
Summary
sessions.jsonfiles and into SQLite-backed session stores.Release Notes
Verification
.agents/skills/autoreview/scripts/autoreview --mode localclean: no accepted/actionable findings reported.pnpm plugin-sdk:api:checkpnpm buildnode scripts/run-vitest.mjs src/gateway/server-methods/agents-mutate.test.ts src/commands/agents.delete.test.ts src/state/openclaw-agent-db.test.ts src/commands/session-state-migration.test.ts src/config/sessions/store-read.test.ts src/commands/sessions-cleanup.test.ts src/commands/tasks.test.ts src/commands/status.summary.test.ts src/commands/status.test.ts