starred/cli-continues

Fork 0

mirror of https://github.com/yigitkonur/cli-continues.git synced 2026-04-26 00:36:02 +03:00

[PR #28] [MERGED] fix: respect CLAUDE_CONFIG_DIR by invalidating stale index cache #28

New issue

Closed

opened 2026-03-04 01:39:20 +03:00 by kerem · 0 comments

kerem commented

2026-03-04 01:39:20 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/yigitkonur/cli-continues/pull/28
Author: @yigitkonur
Created: 3/2/2026
Status: ✅ Merged
Merged: 3/2/2026
Merged by: @yigitkonur

Base: main ← Head: fix/env-var-cache-invalidation

📝 Commits (2)

0ce9046 fix: deduplicate env vars and hash fingerprint for privacy
6c238f9 fix: update tests for dedup+hash fingerprint

📊 Changes

4 files changed (+87 additions, -85 deletions)

View changed files

📝 src/__tests__/env-cache-invalidation.test.ts (+13 -5)
📝 src/parsers/qwen-code.ts (+56 -71)
📝 src/utils/index.ts (+6 -2)
📝 src/utils/resume.ts (+12 -7)

📄 Description

closes #18

hey @yondifon — tracked this down to the session index cache at ~/.continues/sessions.jsonl. it has a 5-min ttl but never checked whether env vars like CLAUDE_CONFIG_DIR changed between runs. so when you ran with a custom config dir, it just served the old cached sessions from ~/.claude/ and never actually scanned your ~/.claude-work/ directory.

what this fixes:

stores an env fingerprint as the first line of the index file
indexNeedsRebuild() now compares that fingerprint against the current env
if any adapter-related env var changes (CLAUDE_CONFIG_DIR, GEMINI_CLI_HOME, XDG_DATA_HOME, etc.), the cache is invalidated and a fresh scan runs

files changed:

src/utils/index.ts — core fix (fingerprint compute + check + store)
src/__tests__/env-cache-invalidation.test.ts — regression test

build passes, all 598 relevant tests pass.

Review all of them with eye of John Carmack-like simplicity with elegeance approach and apply the one only if required

Greptile Summary

Fixes issue #18 by adding an env fingerprint as the first line of the session index (sessions.jsonl). indexNeedsRebuild() now reads the stored fingerprint and compares it against the current env vars from the adapter registry — if they differ, the cache is invalidated. The change is architecturally sound: it correctly derives tracked env vars from the registry rather than a hardcoded list.

Issues found:

FD leak in readStoredFingerprint (line 38–52): the raw openSync/readSync/closeSync pattern leaks the file descriptor if readSync throws before closeSync is reached. readFileSync is simpler, handles cleanup automatically, and is already the idiom used everywhere else in this file.
Duplicate env var keys in fingerprint (line 22–31): opencode and amp both declare envVar: 'XDG_DATA_HOME'; gemini and antigravity both declare envVar: 'GEMINI_CLI_HOME'. Without deduplication, each shared var appears twice in the fingerprint string. Cache invalidation still triggers correctly (symmetric duplicates), but the fingerprint is misleading and will silently accumulate more duplicates as new adapters are added.
Tests don't cover production code (env-cache-invalidation.test.ts): the key "different fingerprints" test reimplements computeEnvFingerprint with a hardcoded 3-adapter subset instead of importing the real function. indexNeedsRebuild() is never called. The regression from issue #18 could be reintroduced and all tests would still pass.

Confidence Score: 3/5

Safe to merge with caveats — the core logic is correct but two implementation bugs should be fixed first.
The approach is correct and registry-derived. The fd leak is a real but low-probability bug (only triggers if readSync fails, which is unusual). The duplicate env var issue doesn't break correctness. Most concerning is that the test suite gives false confidence — it tests a hand-rolled reimplementation, not the production functions.
src/utils/index.ts (fd leak + duplicate envVar keys), src/tests/env-cache-invalidation.test.ts (tests don't import production code)

Important Files Changed

Filename	Overview
src/utils/index.ts	Adds env fingerprint storage and comparison for cache invalidation. Two bugs: fd leak in readStoredFingerprint if readSync throws, and duplicate envVar keys in fingerprint for shared vars (XDG_DATA_HOME, GEMINI_CLI_HOME). Logic is otherwise sound.
src/tests/env-cache-invalidation.test.ts	Tests don't import or exercise the production functions — the critical "fingerprint" test reimplements computeEnvFingerprint with a different hardcoded adapter list. The regression could re-emerge with all tests passing.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[buildIndex called] --> B{force = true?}
    B -- yes --> E[Run all parsers via allSettled]
    B -- no --> C[indexNeedsRebuild?]
    C --> D{statSync INDEX_FILE}
    D -- throws --> E
    D -- ok --> F{age > TTL?}
    F -- yes --> E
    F -- no --> G[readStoredFingerprint]
    G --> H[computeEnvFingerprint\nfrom adapters with envVar]
    H --> I{stored == current?}
    I -- no --> E
    I -- yes --> J[loadIndex from cache\nskipping #env: line]
    E --> K[Sort sessions]
    K --> L[Write fingerprint +\nsessions to INDEX_FILE]
    L --> M[Return sessions]
    J --> M

_{Last reviewed commit: edad1d1}

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/yigitkonur/cli-continues/pull/28 **Author:** [@yigitkonur](https://github.com/yigitkonur) **Created:** 3/2/2026 **Status:** ✅ Merged **Merged:** 3/2/2026 **Merged by:** [@yigitkonur](https://github.com/yigitkonur) **Base:** `main` ← **Head:** `fix/env-var-cache-invalidation` --- ### 📝 Commits (2) - [`0ce9046`](https://github.com/yigitkonur/cli-continues/commit/0ce9046ed691a5031398af4ac0c43b67ed8bb510) fix: deduplicate env vars and hash fingerprint for privacy - [`6c238f9`](https://github.com/yigitkonur/cli-continues/commit/6c238f986819e15293e17323da1cb558bf8383f4) fix: update tests for dedup+hash fingerprint ### 📊 Changes **4 files changed** (+87 additions, -85 deletions) <details> <summary>View changed files</summary> 📝 `src/__tests__/env-cache-invalidation.test.ts` (+13 -5) 📝 `src/parsers/qwen-code.ts` (+56 -71) 📝 `src/utils/index.ts` (+6 -2) 📝 `src/utils/resume.ts` (+12 -7) </details> ### 📄 Description closes #18 hey @yondifon — tracked this down to the session index cache at `~/.continues/sessions.jsonl`. it has a 5-min ttl but never checked whether env vars like `CLAUDE_CONFIG_DIR` changed between runs. so when you ran with a custom config dir, it just served the old cached sessions from `~/.claude/` and never actually scanned your `~/.claude-work/` directory. **what this fixes:** - stores an env fingerprint as the first line of the index file - `indexNeedsRebuild()` now compares that fingerprint against the current env - if any adapter-related env var changes (`CLAUDE_CONFIG_DIR`, `GEMINI_CLI_HOME`, `XDG_DATA_HOME`, etc.), the cache is invalidated and a fresh scan runs **files changed:** - `src/utils/index.ts` — core fix (fingerprint compute + check + store) - `src/__tests__/env-cache-invalidation.test.ts` — regression test build passes, all 598 relevant tests pass.  --- <a href="https://app.devin.ai/review/yigitkonur/cli-continues/pull/28" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a>   # Review all of them with eye of John Carmack-like simplicity with elegeance approach and apply the one only if required <h3>Greptile Summary</h3> Fixes issue #18 by adding an env fingerprint as the first line of the session index (`sessions.jsonl`). `indexNeedsRebuild()` now reads the stored fingerprint and compares it against the current env vars from the adapter registry — if they differ, the cache is invalidated. The change is architecturally sound: it correctly derives tracked env vars from the registry rather than a hardcoded list. **Issues found:** - **FD leak in `readStoredFingerprint`** (line 38–52): the raw `openSync/readSync/closeSync` pattern leaks the file descriptor if `readSync` throws before `closeSync` is reached. `readFileSync` is simpler, handles cleanup automatically, and is already the idiom used everywhere else in this file. - **Duplicate env var keys in fingerprint** (line 22–31): `opencode` and `amp` both declare `envVar: 'XDG_DATA_HOME'`; `gemini` and `antigravity` both declare `envVar: 'GEMINI_CLI_HOME'`. Without deduplication, each shared var appears twice in the fingerprint string. Cache invalidation still triggers correctly (symmetric duplicates), but the fingerprint is misleading and will silently accumulate more duplicates as new adapters are added. - **Tests don't cover production code** (`env-cache-invalidation.test.ts`): the key "different fingerprints" test reimplements `computeEnvFingerprint` with a hardcoded 3-adapter subset instead of importing the real function. `indexNeedsRebuild()` is never called. The regression from issue #18 could be reintroduced and all tests would still pass. <details><summary><h3>Confidence Score: 3/5</h3></summary> - Safe to merge with caveats — the core logic is correct but two implementation bugs should be fixed first. - The approach is correct and registry-derived. The fd leak is a real but low-probability bug (only triggers if readSync fails, which is unusual). The duplicate env var issue doesn't break correctness. Most concerning is that the test suite gives false confidence — it tests a hand-rolled reimplementation, not the production functions. - src/utils/index.ts (fd leak + duplicate envVar keys), src/__tests__/env-cache-invalidation.test.ts (tests don't import production code) </details> <details open><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | src/utils/index.ts | Adds env fingerprint storage and comparison for cache invalidation. Two bugs: fd leak in readStoredFingerprint if readSync throws, and duplicate envVar keys in fingerprint for shared vars (XDG_DATA_HOME, GEMINI_CLI_HOME). Logic is otherwise sound. | | src/__tests__/env-cache-invalidation.test.ts | Tests don't import or exercise the production functions — the critical "fingerprint" test reimplements computeEnvFingerprint with a different hardcoded adapter list. The regression could re-emerge with all tests passing. | </details> </details> <h3>Flowchart</h3> ```mermaid %%{init: {'theme': 'neutral'}}%% flowchart TD A[buildIndex called] --> B{force = true?} B -- yes --> E[Run all parsers via allSettled] B -- no --> C[indexNeedsRebuild?] C --> D{statSync INDEX_FILE} D -- throws --> E D -- ok --> F{age > TTL?} F -- yes --> E F -- no --> G[readStoredFingerprint] G --> H[computeEnvFingerprint\nfrom adapters with envVar] H --> I{stored == current?} I -- no --> E I -- yes --> J[loadIndex from cache\nskipping #env: line] E --> K[Sort sessions] K --> L[Write fingerprint +\nsessions to INDEX_FILE] L --> M[Return sessions] J --> M ``` <sub>Last reviewed commit: edad1d1</sub>   --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

kerem

2026-03-04 01:39:20 +03:00

closed this issue
added the
pull-request
label

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/cli-continues#28

No description provided.

Rows
Columns