google-gemini / google-gemini/gemini-cli
bug: session retention can delete unrelated sessions on short-ID collision
- Dominant language
- TypeScript
- Stars
- 107k
- Forks
- 14.6k
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 45
Description
## What happened?
Session retention cleanup groups files by the eight-character ID suffix in their filenames. Once any file in that group is selected for deletion, every file with the same suffix is deleted, even when another file contains a different full session ID and is not expired.
The relevant flow is in `packages/cli/src/utils/sessionCleanup.ts`:
- Lines 141-145 correctly compute the individual sessions that should be deleted.
- Lines 147-167 derive an eight-character `shortId` from one selected session and find every filename ending in that suffix.
- Lines 169-223 delete all those matching files. The full ID read from each file is used only to preserve the current session, not to verify that the file belongs to the expired session family or independently satisfies retention.
The existing test at `packages/cli/src/utils/sessionCleanup.test.ts:446` intentionally verifies deletion of a parent and subagent sharing a short ID, but it does not cover an unrelated recent session whose UUID has the same first eight characters.
## How can this be reproduced?
1. Create an expired session whose filename ends in `-deadbeef.jsonl` and whose full session ID begins with `deadbeef`.
2. Create a recent, unrelated session with a different full UUID that also begins with `deadbeef`, producing another filename ending in `-deadbeef.jsonl`.
3. Enable age-based session retention and run `cleanupExpiredSessions()`.
4. The expired file enters `sessionsToDelete`; its short ID then selects both files, and both are unlinked.
This can be reproduced deterministically in a unit test by using synthetic IDs; no probabilistic collision is required in the test.
## What did you expect to happen?
Only sessions independently selected by the retention policy, plus subagent/artifact records proven to belong to those sessions, should be deleted. An unrelated recent session must survive a short-ID collision.
## Why this matters
This is a user-data-loss path. Eight hexadecimal characters provide only 32 bits of namespace, so collisions become increasingly plausible across long-lived or heavily automated projects. Cleanup also deletes associated artifacts after reading the unrelated file's full ID.
## Suggested direction
- Use the full persisted session ID and explicit parent/subagent relationship when building deletion groups.
- Do not treat a filename short-ID match alone as proof of ownership.
- Add a regression test containing an expired session and an unrelated recent session with the same filename suffix.
## Client information
Found by source audit on current `main`, commit `f47d6c6f7`. This is platform-independent filesystem logic; `/about` output is not applicable.
Contributor guide
Research direction
Start in packages/cli/src/utils/sessionCleanup.ts, especially lines 141-223, and read the existing case at packages/cli/src/utils/sessionCleanup.test.ts:446. Add a deterministic regression test with an expired session and an unrelated recent session sharing an eight-character filename suffix, then run the session cleanup tests. Done means only sessions selected by retention and proven related records are deleted.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- cli
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100