Skip to content

Reconcile cache with filesystem on get-projects so sessions stop going missing - #29

Merged
JeanBaptisteRenard merged 1 commit into
devsuitup:mainfrom
ymajoros:fix/reconcile-cache-jbr
Jun 4, 2026
Merged

Reconcile cache with filesystem on get-projects so sessions stop going missing#29
JeanBaptisteRenard merged 1 commit into
devsuitup:mainfrom
ymajoros:fix/reconcile-cache-jbr

Conversation

@ymajoros

@ymajoros ymajoros commented Jun 1, 2026

Copy link
Copy Markdown

Problem

Sessions and whole worktrees silently vanish from the sidebar while their transcripts are still on disk, and reopening/restarting doesn't bring them back.

get-projects only scans the filesystem on a cold start:

const needsPopulate = !isCachePopulated() || !isSearchIndexPopulated();
if (needsPopulate) { await populateCacheViaWorker(); }
return buildProjectsFromCache(showArchived);   // otherwise: cache only, never reconciled

Once the cache is non-empty it's never reconciled again, so folders that changed while the app was closed — or that were never indexed by the build that first populated the cache (cache_meta.indexMtimeMs === 0) — never reappear. The live fs.watch only covers changes while the app is running.

Fix

When the cache is already populated, reconcile before building from cache. The dead populateCacheFromFilesystem() (defined + exported, never called) is repurposed into reconcileCacheFromFilesystem(), now stat-gated: it re-indexes only folders that are new or whose newest .jsonl is newer than what was last indexed.

if (needsPopulate) {
  await populateCacheViaWorker();
} else {
  reconcileCacheFromFilesystem();
}
for (const folder of folders) {
  const meta = metaMap.get(folder);
  const folderPath = path.join(PROJECTS_DIR, folder);
  if (!meta || getFolderIndexMtimeMs(folderPath) > (meta.indexMtimeMs || 0)) {
    refreshFolder(folder);
  }
}

When nothing changed this is just a readdir + stat per folder — no transcript parsing, no DB writes — so steady-state cost is negligible; real work happens only for folders that need it. refreshFolder already does its own per-file mtime skip, so this composes with the targeted-refresh path you added.

Testing

  • Added test/reconcile-cache.test.js asserting the gate: a never-indexed folder and a stale folder (indexMtimeMs older than disk) get indexed; an up-to-date folder is skipped. Passes under node --test test/reconcile-cache.test.js (also ran derive-project-path + folder-index-state green). I didn't run the dom-* tests here (need jsdom/devDeps installed).
  • Verified live on Linux / 0.0.30: 27 worktree folders stuck empty (indexMtimeMs = 0) re-indexed and reappeared.

Notes

@JeanBaptisteRenard JeanBaptisteRenard left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks a lot for this PR @ymajoros — and for the two others! The diagnosis is spot on (get-projects only scanning the filesystem on cold start is exactly why sessions were vanishing), the reconcile approach is the right fix, and the test is well-structured. Much appreciated. 🙏

I ran the full suite locally on your branch: 102/102 pass. Two things before merge:

1. Please rebase onto current main (required)

Your branch is based on 9dd4a66, which predates 13219eb (fix(stats): wire replaceSessionMetrics + touchCachedModified into sessionCache ctx.db). On your base, the ctx.db literal in main.js is missing touchCachedModified and replaceSessionMetrics, which session-cache.js calls in refreshFolder's header-read fallback path. Calling undefined() there throws a TypeError that is silently swallowed by refreshFolder's try/catch — and your reconcile pass calls refreshFolder on every stale folder, so it widens the blast radius of that (now-fixed-on-main) bug.

A rebase onto current main picks up the wiring and resolves this — no code change needed on your side, just please double-check after rebasing that the sessionCache.init({ db: { ... } }) literal includes both functions.

(Context: this is a known pitfall on this fork — any db.js function used by session-cache.js must be whitelisted in the ctx.db literal in main.js, otherwise the worker fails silently.)

2. Reconcile gate misses offline subagent changes (should fix, or scope-exclude)

The gate compares getFolderIndexMtimeMs(folderPath) against meta.indexMtimeMs, but getFolderIndexMtimeMs only stats top-level *.jsonl — it doesn't descend into subagents/. A folder where only subagent transcripts changed while the app was closed won't be flagged, so those changes stay missing (the exact scenario this PR targets). Once a folder is flagged, refreshFolder correctly uses enumerateSessionFiles which does recurse — the gap is only in the gate condition.

Suggestion: extend getFolderIndexMtimeMs to also stat subagents/*.jsonl, and add a regression test for the subagent-only offline change (your test file structure makes that easy to add). If you'd rather keep the scope tight, a note in the PR description acknowledging the limitation works too, and we'll follow up.

Minor / non-blocking

  • loadProjects in the renderer calls getProjects(false) and getProjects(true) via Promise.all, so the reconcile pass runs twice per sidebar load. Harmless (it's cheap and sync), just worth a dedupe flag someday.

Verdict: request changes — but really just the rebase + a decision on the subagent gate. Happy to merge right after. Thanks again!

…g missing

The session cache is only rebuilt from the filesystem on a cold start
(populateCacheViaWorker runs only when the cache is completely empty). Once it
has any rows, a project folder that changed while the app was closed — or that
predates the build which first indexed it — is never re-scanned, so its
sessions (and whole worktrees) silently disappear from the sidebar. The live
fs.watch only covers changes while the app is running.

When the cache is already populated, get-projects now reconciles first: the
dead populateCacheFromFilesystem() (defined + exported, never called) becomes a
stat-gated reconcileCacheFromFilesystem() that re-indexes only folders that are
new or whose newest transcript is newer than cache_meta.indexMtimeMs — cheap
when nothing changed.

Also extend getFolderIndexMtimeMs to stat the full session-file set
(enumerateSessionFiles), not just top-level *.jsonl, so a folder whose only
offline change was a subagent transcript under <id>/subagents/ is still flagged
for reconcile. Adds regression tests for the gate and the subagent-only case.

Rebased onto current main, which wires touchCachedModified/replaceSessionMetrics
into the ctx.db literal.
@ymajoros
ymajoros force-pushed the fix/reconcile-cache-jbr branch from c1da4cb to 51924a2 Compare June 4, 2026 13:11
@ymajoros

ymajoros commented Jun 4, 2026

Copy link
Copy Markdown
Author

Thanks for the thorough review! Addressed both blocking points — force-pushed.

1. Rebased onto current main. Now based on 4724527, so it picks up the replaceSessionMetrics/touchCachedModified wiring in the ctx.db literal — the swallowed-TypeError hazard in refreshFolder's header-read fallback is gone. Verified sessionCache.init({ db: { … } }) includes both.

2. Subagent gate. Good catch. getFolderIndexMtimeMs now stats the full enumerateSessionFiles(folderPath) set (top-level and <id>/subagents/ transcripts) instead of only top-level *.jsonl — same enumeration refreshFolder uses, so the gate covers exactly what a refresh would scan. Added a regression test (folder index timestamp advances when ONLY a subagent transcript changes) plus the reconcile gate test (test/reconcile-cache.test.js). node --test green on both.

3. (non-blocking) Yep — loadProjects does getProjects(false)+getProjects(true), so reconcile runs twice per sidebar load. It's stat-gated (a readdir+stat sweep, no DB writes when nothing changed), so it's cheap; happy to add a throttle/once-flag in a follow-up if you'd like.

@JeanBaptisteRenard JeanBaptisteRenard left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed the force-pushed version — both blocking points are cleanly addressed:

  • ✅ Rebase picks up the ctx.db wiring (touchCachedModified + replaceSessionMetrics verified present in the literal)
  • getFolderIndexMtimeMs now walks enumerateSessionFiles — same enumeration as refreshFolder, so the gate covers exactly what a refresh would scan, subagents included. The regression test is exactly what I hoped for.

Local run: 121/121 tests pass, CI green on Node 20/22.

Merging. I'll push a tiny follow-up PR for the two non-blocking notes (a comment documenting the intentionally-await-less sync call, and a small throttle for the double sweep from loadProjects's twin get-projects calls) — no action needed on your side.

Thanks a lot for the quality of this one, and for the fast, thorough turnaround on the review. Sessions silently vanishing was one of the most annoying bugs on this fork — great to have it nailed. 🙏

@JeanBaptisteRenard
JeanBaptisteRenard merged commit abb4950 into devsuitup:main Jun 4, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants