google-gemini / google-gemini/gemini-cli

bug: session retention can delete unrelated sessions on short-ID collision

Open
#28,643 0 comments 0 reactions 0 assignees View on GitHub
area/agent status/need-triage
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.