Skip to content

Fix session index reconciliation and stuck loading state#72

Merged
davidliuk merged 3 commits intomainfrom
fix/session-index-reconcile-and-stuck-loading
Mar 22, 2026
Merged

Fix session index reconciliation and stuck loading state#72
davidliuk merged 3 commits intomainfrom
fix/session-index-reconcile-and-stuck-loading

Conversation

@bbsngg
Copy link
Copy Markdown
Contributor

@bbsngg bbsngg commented Mar 22, 2026

Summary

  • split session index placeholder writes from source-of-truth reconciliation so Claude, Gemini, and Codex sessions can refresh names, counts, timestamps, and real IDs safely
  • move Claude completion cleanup off the critical path and reconcile single-session files instead of rescanning an entire project before sending claude-complete
  • stop the chat UI from treating stale Resuming... timer state as an active run, while keeping a short resume/status-check grace window

Testing

  • node --check server/projects.js
  • node --check server/claude-sdk.js
  • npm run typecheck
  • validated stale Claude session metadata is reconciled from real .jsonl data in the local DB
  • validated affected session records now show synced names/counts/timestamps in session_metadata

bbsngg
bbsngg commented Mar 22, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@bbsngg bbsngg left a comment

Choose a reason for hiding this comment

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

PR Review Summary

Overall this is a solid improvement that addresses real UX issues (stuck loading) and data consistency problems (placeholder vs source-of-truth separation). The architecture direction is sound. A few items worth addressing:

Issues

1. Dead code in reconcileClaudeSessionIndex (server/projects.js)
When targetSessionId is null, the function calls getSessions(projectName, 0, 0) then immediately hits if (!targetSessionId) return result; — the code block below it (lines ~708-711 with the .session field) is unreachable.

2. Duplicated timestamp normalization logic

  • toIsoString() in projects.js duplicates normalizeSessionTimestamp() in db.js
  • normalizeSessionTimestamp and normalizeSessionCreatedAt in db.js are nearly identical (only difference is the replace('T', ' ').slice(0, 19) formatting)

Consider consolidating to avoid behavioral drift.

3. Hardcoded magic values

  • The 5000ms timeout in useChatSessionState.ts for clearing stale loading state should be extracted to a named constant with a comment explaining the chosen duration. If WebSocket reconnection takes longer than 5s, this could prematurely clear a legitimately active session's loading state.
  • The string 'Resuming...' appears in both ChatInterface.tsx and useChatSessionState.ts — should be a shared constant.

4. Sequential awaits in Gemini/Codex reconcile loops (server/projects.js)
The for...of + await pattern in getGeminiSessions and getCodexSessions sync paths will serialize DB writes. Consider Promise.all/Promise.allSettled for parallelism when session count is non-trivial.

Minor / Non-blocking

  • Sidebar sort order change (lastActivity || createdAt instead of createdAt || lastActivity) for codex/gemini is a behavior change — makes sense (sort by recent activity), just confirming it's intentional.
  • setTimeout(async () => ...) in scheduleClaudeSessionIndexReconcile — the try/catch covers it, but unhandled rejections inside setTimeout callbacks can be subtle; current code is fine.
  • Error-path reconcile in claude-sdk.js catch block uses Promise.allSettled which silently swallows reconcile failures — acceptable since it's best-effort cleanup.

Verdict

Good to merge after addressing items 1-3. Item 4 is a nice-to-have optimization.

@bbsngg @claude
Address PR review: remove dead code, consolidate timestamp utils, ext…
15689d0 
…ract constants, parallelize reconcile

- Remove unreachable code block in reconcileClaudeSessionIndex
- Remove duplicate toIsoString() from projects.js, reuse normalizeSessionTimestamp from db.js
- Extract RESUMING_STATUS_TEXT constant and STATUS_VALIDATION_TIMEOUT_MS constant
- Parallelize Gemini/Codex syncIndex loops with Promise.allSettled
- Add comment explaining normalizeSessionCreatedAt format difference

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@bbsngg
Copy link
Copy Markdown
Contributor Author

bbsngg commented Mar 22, 2026

Pushed 15689d0 to address the review feedback:

  1. Dead code removed — cleaned up unreachable if (!targetSessionId) guard and dead return in reconcileClaudeSessionIndex
  2. Timestamp normalization consolidated — removed toIsoString() from projects.js, now reuses normalizeSessionTimestamp exported from db.js; added comment to normalizeSessionCreatedAt explaining the YYYY-MM-DD HH:MM:SS format for SQLite convention
  3. Magic values extracted — RESUMING_STATUS_TEXT shared constant in types.ts (replaced 5 occurrences across 3 files), STATUS_VALIDATION_TIMEOUT_MS constant with doc comment in useChatSessionState.ts
  4. Reconcile loops parallelized — Gemini and Codex syncIndex blocks now use Promise.allSettled instead of sequential for...of

All checks pass: node --check server/projects.js, node --check server/database/db.js, npm run typecheck.

@davidliuk
Copy link
Copy Markdown
Collaborator

davidliuk commented Mar 22, 2026

Thanks for the fix. I found two issues worth addressing before merge:

High

  1. check-session-status can now retrigger itself in a loop. In useChatSessionState, the effect sends check-session-status whenever it runs, but it also depends on pendingStatusValidationSessionId, and that same state is toggled by the status response path via resolveSessionStatusCheck. That creates a cycle of pending -> send status check -> resolve pending -> effect reruns -> send status check again, which can cause repeated WebSocket status checks and unnecessary message reloads for the currently selected session. Relevant spots: src/components/chat/hooks/useChatSessionState.ts and src/components/chat/view/ChatInterface.tsx.

Medium

  1. Codex/Gemini completion is still blocked on full session-index reconciliation. In both backends, reconcileCodexSessionIndex(...) / reconcileGeminiSessionIndex(...) run before codex-complete / gemini-complete is emitted. Those reconcile paths still go through the full provider session discovery/index rebuild flow before filtering to the target session, so on installations with many historical sessions the UI can remain loading after the model has already finished. Relevant spots: server/openai-codex.js, server/gemini-cli.js, and the reconcile helpers in server/projects.js.

I did not see other obvious correctness issues, and local typecheck passes. The main missing coverage is regression testing for these two paths: preventing repeated check-session-status sends after session selection/reconnect, and ensuring Codex/Gemini completion is not delayed by index reconciliation.

@bbsngg @claude
Fix check-session-status retrigger loop and unblock Codex/Gemini comp…
33a967e 
…letion

- Break status-check loop by using a ref for pendingStatusValidationSessionId
  in the loadMessages effect, removing it from the dependency array so that
  resolving the pending status no longer re-triggers the effect
- Move codex-complete emission before reconcileCodexSessionIndex so the UI
  settles immediately; reconciliation runs as post-completion housekeeping
- Move gemini-complete emission before reconcileGeminiSessionIndex, same pattern

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@bbsngg
Copy link
Copy Markdown
Contributor Author

bbsngg commented Mar 22, 2026

Pushed 33a967e to address the two issues from the latest review:

High: check-session-status retrigger loop

Root cause: The main loadMessages effect in useChatSessionState.ts had pendingStatusValidationSessionId in its dependency array. When the WebSocket response resolved the pending status (setting it to null), the effect re-ran, called markSessionStatusCheckPending again, and sent another check-session-status — creating an infinite cycle.

Fix: Introduced a ref (pendingStatusValidationSessionIdRef) that mirrors the state value. The loadMessages effect now reads the ref instead of the state directly, and pendingStatusValidationSessionId is removed from the dependency array. This breaks the cycle: resolving the pending status no longer re-triggers the effect, while the ref still provides the current value for the isProcessing check.

Medium: Codex/Gemini completion blocked by reconciliation

Root cause: Both openai-codex.js and gemini-cli.js awaited reconcileCodexSessionIndex / reconcileGeminiSessionIndex before emitting the completion event. These reconcile paths scan all session files on disk before filtering to the target, causing UI to remain loading after the model finishes.

Fix: Reordered to emit codex-complete / gemini-complete before reconciliation, matching the existing Claude SDK pattern (claude-complete is already emitted first, with reconciliation as post-completion housekeeping). Reconciliation still runs and completes, but the UI receives the completion signal immediately.

Testing notes

  • npm run typecheck passes
  • node --check server/openai-codex.js and node --check server/gemini-cli.js pass
  • Remaining manual verification: confirm no repeated check-session-status on session selection/reconnect, and confirm Codex/Gemini UI exits loading promptly on model completion

@davidliuk davidliuk merged commit 8f0af41 into main Mar 22, 2026
1 check passed
@bbsngg bbsngg deleted the fix/session-index-reconcile-and-stuck-loading branch March 22, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bbsngg @davidliuk