Skip to content

Usage: include reset and deleted session archives#43215

Merged
frankekn merged 5 commits intoopenclaw:mainfrom
rcrick:fix/42032-usage-include-archived-sessions
Mar 23, 2026
Merged

Usage: include reset and deleted session archives#43215
frankekn merged 5 commits intoopenclaw:mainfrom
rcrick:fix/42032-usage-include-archived-sessions

Conversation

@rcrick
Copy link
Contributor

@rcrick rcrick commented Mar 11, 2026

Summary

  • Problem: Usage aggregation only counted active *.jsonl session transcripts and skipped archived *.jsonl.reset.* / *.jsonl.deleted.* transcripts.
  • Why it matters: Usage page totals become much lower than actual spend after /new, session reset, or deleting old sessions.
  • What changed: usage scanning and session discovery now include reset and deleted transcript archives, and per-session usage detail falls back to archived transcripts when the active file no longer exists.
  • What did NOT change (scope boundary): *.jsonl.bak.* archives are still excluded to avoid double-counting backup copies.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Memory / storage
  • API / contracts
  • UI / DX
  • Skills / tool execution
  • Auth / tokens
  • Integrations
  • CI/CD / infra

Linked Issue/PR

User-visible / Behavior Changes

  • Usage totals now include spend from archived reset and deleted session transcripts.
  • Session usage listing can discover archived reset/deleted sessions.
  • Per-session usage detail can still load logs/timeseries/summary after the active transcript has been rotated away.

Security Impact (required)

  • New permissions/capabilities? (No)
  • Secrets/tokens handling changed? (No)
  • New/changed network calls? (No)
  • Command/tool execution surface changed? (No)
  • Data access scope changed? (No)
  • If any Yes, explain risk + mitigation:

Repro + Verification

Environment

  • OS: macOS
  • Runtime/container: Node 24 / pnpm
  • Model/provider: N/A
  • Integration/channel (if any): Control UI usage aggregation
  • Relevant config (redacted): OPENCLAW_STATE_DIR pointed at temp test state dirs

Steps

  1. Create session transcripts for active *.jsonl, archived *.jsonl.reset.*, archived *.jsonl.deleted.*, and backup *.jsonl.bak.*.
  2. Run usage aggregation and session discovery against that state directory.
  3. Query per-session usage detail for a session that only exists as an archived reset transcript.

Expected

  • Active, reset, and deleted transcripts are counted in usage totals.
  • Backup bak archives are not counted.
  • Per-session usage detail still works when only the archived transcript remains.

Actual

  • Before this change, only active *.jsonl transcripts were counted, so reset/deleted usage was omitted.

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Targeted tests run:

corepack pnpm vitest run src/config/sessions/artifacts.test.ts src/infra/session-cost-usage.test.ts src/gateway/server-methods/usage.sessions-usage.test.ts

Result: 23 tests passed.

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios: counted active/reset/deleted transcripts in aggregate usage; excluded bak; loaded summary/timeseries/logs from archived reset transcript fallback.
  • Edge cases checked: archived session discovery still derives the original session id; specific session usage lookup still resolves when the active file is gone.
  • What you did not verify: full manual Control UI click-through against a live local gateway.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

  • Backward compatible? (Yes)
  • Config/env changes? (No)
  • Migration needed? (No)
  • If yes, exact upgrade steps:

Failure Recovery (if this breaks)

  • How to disable/revert this change quickly: revert this PR/commit to restore the previous active-transcript-only behavior.
  • Files/config to restore: src/config/sessions/artifacts.ts, src/infra/session-cost-usage.ts, src/gateway/server-methods/usage.ts
  • Known bad symptoms reviewers should watch for: usage totals unexpectedly jumping from counting backup archives, or archived session detail lookups returning duplicate/incorrect sessions.

Risks and Mitigations

  • Risk: counting archived files could accidentally include backup copies and double-count usage.
    • Mitigation: only reset and deleted archives are included; bak remains excluded, with test coverage for that boundary.

@openclaw-barnacle openclaw-barnacle bot added gateway Gateway runtime size: M labels Mar 11, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 11, 2026

Greptile Summary

This PR fixes a real under-counting bug: usage aggregation and session discovery previously ignored *.jsonl.reset.* and *.jsonl.deleted.* archive transcripts, causing the Usage page to show totals much lower than actual spend after a session reset or deletion. The fix is well-scoped — *.jsonl.bak.* files remain excluded to prevent double-counting — and the new helper functions (isUsageCountedSessionTranscriptFileName, parseUsageCountedSessionIdFromFileName) are clean and well-tested.

Two logic issues need attention before merging:

  • Duplicate sessions in list view after a reset (src/infra/session-cost-usage.ts, discoverAllSessions): After /reset, both sess.jsonl (new active file) and sess.jsonl.reset.<TIMESTAMP> (archived pre-reset file) exist on disk simultaneously. discoverAllSessions now emits both as separate DiscoveredSession entries with the same sessionId, and no downstream deduplication occurs, so the same session appears twice in the UI session list.

  • Wrong archive selected when both reset and deleted archives exist (resolveExistingUsageSessionFile, line 335–338): The "latest archive" is chosen by a full-filename lexicographic sort. Because 'r' > 'd' in ASCII, a .reset. archive always sorts above a .deleted. archive, even when the deleted archive has a later timestamp (e.g., reset at T11:00, deleted at T12:00). The comparator should sort only on the timestamp suffix.

Confidence Score: 3/5

  • PR introduces two logic bugs — duplicate session entries in list view after reset, and incorrect archive selection when reset+deleted archives coexist — that should be addressed before merging.
  • The core concept and implementation are sound, and the new artifact helpers are correct. However, two logic bugs were found: (1) discoverAllSessions emits duplicate sessionId entries when an active file and a reset archive coexist, causing UI duplicates; (2) resolveExistingUsageSessionFile's lexicographic sort picks a reset archive over a newer deleted archive. Neither bug was caught by the new tests, which don't exercise the coexistence scenario. Score of 3 reflects meaningful but fixable issues that affect correctness in real post-reset workflows.
  • src/infra/session-cost-usage.ts — specifically discoverAllSessions (deduplication by sessionId) and resolveExistingUsageSessionFile (archive sort comparator).

Comments Outside Diff (1)

  1. src/infra/session-cost-usage.ts, line 449-517 (link)

    Duplicate session IDs when active transcript and reset archive coexist

    After a /reset, the filesystem holds both sess.jsonl (the new, post-reset active file) and sess.jsonl.reset.<TIMESTAMP> (the archived pre-reset transcript). discoverAllSessions now includes both files, and parseUsageCountedSessionIdFromFileName maps them to the same sessionId ("sess"). Because there is no deduplication by sessionId inside this loop, the result contains two DiscoveredSession entries with identical sessionId values.

    In discoverAllSessionsForUsage (also not deduplicated) and then in the usage.ts list-view merge loop, both entries survive and are emitted to mergedEntries, causing the same session to appear twice in the UI session list.

    The test added in this PR only covers the case where only the archived transcript exists (no active file), so it doesn't catch this scenario.

    A fix would be to deduplicate by sessionId in discoverAllSessions, keeping the entry whose sessionFile is the most recently modified:

    // After building `discovered`, deduplicate by sessionId keeping newest mtime
    const bySessionId = new Map<string, DiscoveredSession>();
    for (const entry of discovered) {
      const existing = bySessionId.get(entry.sessionId);
      if (!existing || entry.mtime > existing.mtime) {
        bySessionId.set(entry.sessionId, entry);
      }
    }
    return [...bySessionId.values()].toSorted((a, b) => b.mtime - a.mtime);
    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/infra/session-cost-usage.ts
    Line: 449-517
    
    Comment:
    **Duplicate session IDs when active transcript and reset archive coexist**
    
    After a `/reset`, the filesystem holds *both* `sess.jsonl` (the new, post-reset active file) **and** `sess.jsonl.reset.<TIMESTAMP>` (the archived pre-reset transcript). `discoverAllSessions` now includes both files, and `parseUsageCountedSessionIdFromFileName` maps them to the same `sessionId` ("sess"). Because there is no deduplication by `sessionId` inside this loop, the result contains two `DiscoveredSession` entries with identical `sessionId` values.
    
    In `discoverAllSessionsForUsage` (also not deduplicated) and then in the `usage.ts` list-view merge loop, both entries survive and are emitted to `mergedEntries`, causing the same session to appear twice in the UI session list.
    
    The test added in this PR only covers the case where *only* the archived transcript exists (no active file), so it doesn't catch this scenario.
    
    A fix would be to deduplicate by `sessionId` in `discoverAllSessions`, keeping the entry whose `sessionFile` is the most recently modified:
    
    ```ts
    // After building `discovered`, deduplicate by sessionId keeping newest mtime
    const bySessionId = new Map<string, DiscoveredSession>();
    for (const entry of discovered) {
      const existing = bySessionId.get(entry.sessionId);
      if (!existing || entry.mtime > existing.mtime) {
        bySessionId.set(entry.sessionId, entry);
      }
    }
    return [...bySessionId.values()].toSorted((a, b) => b.mtime - a.mtime);
    ```
    
    How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: 007b637

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 007b6378db

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 79a501581f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@rcrick rcrick force-pushed the fix/42032-usage-include-archived-sessions branch 2 times, most recently from 943fe08 to afa5239 Compare March 11, 2026 15:01
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: afa5239c1d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b167ed4975

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".


for (const entry of entries) {
if (!entry.isFile() || !entry.name.endsWith(".jsonl")) {
if (!entry.isFile() || !isUsageCountedSessionTranscriptFileName(entry.name)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Filter archived sessions by transcript activity window

Including *.jsonl.reset.*/*.jsonl.deleted.* in discovery here makes the existing mtime-based date filter inaccurate for archived transcripts, because archive rename time is often much newer than the messages inside the file. If a user deletes an old session today and then requests a recent range (for example, last 7 days), that archive is discovered even when it has no in-range activity; sessions.usage applies limit before loading per-session usage, so these zero-usage rows can displace real in-range sessions. This path should gate archive inclusion on transcript timestamps (or drop sessions with no in-range usage before limiting).

Useful? React with 👍 / 👎.

@frankekn frankekn force-pushed the fix/42032-usage-include-archived-sessions branch from b167ed4 to 5cf4c67 Compare March 23, 2026 04:42
@frankekn frankekn force-pushed the fix/42032-usage-include-archived-sessions branch from 5cf4c67 to 49ed6c2 Compare March 23, 2026 04:42
@frankekn frankekn self-assigned this Mar 23, 2026
@frankekn
Copy link
Contributor

@codex review

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. Swish!

ℹ️ 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".

@frankekn frankekn merged commit 2fe1ff8 into openclaw:main Mar 23, 2026
37 of 38 checks passed
@frankekn
Copy link
Contributor

Merged via squash.

Thanks @rcrick!

sliverp pushed a commit to sliverp/openclaw that referenced this pull request Mar 23, 2026
Merged via squash.

Prepared head SHA: 49ed6c2
Co-authored-by: rcrick <23069968+rcrick@users.noreply.github.com>
Co-authored-by: frankekn <4488090+frankekn@users.noreply.github.com>
Reviewed-by: @frankekn
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Usage page doesn't count tokens from reset and delete sessions

2 participants