fix: mark stale ongoing sessions as dead after 5min inactivity#100
fix: mark stale ongoing sessions as dead after 5min inactivity#100
Conversation
Sessions that crash, get killed, or hit usage limits remain marked as "ongoing" because their last JSONL activity is a tool_use/thinking event with no ending event. Add an mtime staleness check — if the file hasn't been written to in 5+ minutes, override isOngoing to false. Closes #94 Co-Authored-By: Claude Opus 4.6 <[email protected]>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where sessions that crash or are terminated prematurely continue to be displayed as "ongoing" because their activity logs lack a definitive ending event. The solution implements a staleness check that re-evaluates the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses the issue of stale 'ongoing' sessions by introducing a 5-minute inactivity timeout based on the file's last modification time. The implementation is clean and efficient as it reuses existing file metadata without additional I/O. I have one suggestion to improve code organization regarding the new constant.
|
|
||
| // If messages suggest ongoing but the file hasn't been written to in 5+ minutes, | ||
| // the session likely crashed/was killed — mark as dead (issue #94) | ||
| const STALE_SESSION_THRESHOLD_MS = 5 * 60 * 1000; |
There was a problem hiding this comment.
For better code organization and to avoid re-declaring this constant on every function call, consider defining STALE_SESSION_THRESHOLD_MS at a higher scope. A good place would be as a module-level constant at the top of the file, near SEARCH_PROJECT_CACHE_TTL_MS.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f21f267d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const isOngoing = | ||
| metadata.isOngoing && Date.now() - effectiveMtime < STALE_SESSION_THRESHOLD_MS; |
There was a problem hiding this comment.
This staleness gate can flip an actually active session to isOngoing = false whenever no JSONL write happens for 5 minutes, which is a valid runtime pattern for long tool calls or long assistant steps (for example, a command that blocks without output). In those cases metadata.isOngoing is still true because no ending event has been written yet, so overriding it with Date.now() - effectiveMtime < 5min causes a false “dead” state until another write arrives.
Useful? React with 👍 / 👎.
📝 WalkthroughWalkthroughImplements a stale-session detection heuristic in the project scanner that marks sessions as dead when the underlying file modification time exceeds a 5-minute threshold, even if the session metadata indicates it is ongoing. Changes
Suggested labels
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/services/discovery/ProjectScanner.ts (1)
757-773: LGTM! Solid implementation of stale session detection.The logic correctly marks sessions as dead when the file hasn't been modified in 5+ minutes while metadata still indicates ongoing activity. Good use of the already-computed
effectiveMtimeto avoid extra I/O, and the inline comment clearly documents the rationale with a reference to issue#94.One optional consideration: extract
STALE_SESSION_THRESHOLD_MSto module level (alongsideSEARCH_PROJECT_CACHE_TTL_MSon line 62) for consistency and easier adjustments/testing.,
🔧 Optional: Extract constant to module level
/** How long to reuse the cached project list for search (ms) */ const SEARCH_PROJECT_CACHE_TTL_MS = 30_000; + +/** Threshold after which an "ongoing" session is considered stale/dead (ms) */ +const STALE_SESSION_THRESHOLD_MS = 5 * 60 * 1000;Then in
buildSessionMetadata:// If messages suggest ongoing but the file hasn't been written to in 5+ minutes, // the session likely crashed/was killed — mark as dead (issue `#94`) - const STALE_SESSION_THRESHOLD_MS = 5 * 60 * 1000; const isOngoing = metadata.isOngoing && Date.now() - effectiveMtime < STALE_SESSION_THRESHOLD_MS;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/services/discovery/ProjectScanner.ts` around lines 757 - 773, Extract the hardcoded STALE_SESSION_THRESHOLD_MS from inside buildSessionMetadata to a module-level constant (next to SEARCH_PROJECT_CACHE_TTL_MS) and replace the local definition with the module-level one; ensure the constant name remains STALE_SESSION_THRESHOLD_MS and that buildSessionMetadata (the function computing isOngoing using effectiveMtime) continues to reference it so behavior is unchanged and tests/configs can reuse/override the threshold easily.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/services/discovery/ProjectScanner.ts`:
- Around line 757-773: Extract the hardcoded STALE_SESSION_THRESHOLD_MS from
inside buildSessionMetadata to a module-level constant (next to
SEARCH_PROJECT_CACHE_TTL_MS) and replace the local definition with the
module-level one; ensure the constant name remains STALE_SESSION_THRESHOLD_MS
and that buildSessionMetadata (the function computing isOngoing using
effectiveMtime) continues to reference it so behavior is unchanged and
tests/configs can reuse/override the threshold easily.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52dd62cd-cb45-495a-a65f-93315b133933
📒 Files selected for processing (1)
src/main/services/discovery/ProjectScanner.ts
1 participant
Summary
ProjectScanner.buildSessionMetadata()— if the session file hasn't been written to in 5+ minutes,isOngoingis overridden tofalseeffectiveMtimeso no additional I/O is neededCloses #94
Test plan
pnpm typecheckpasses🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes