feat: session status hooks (working/idle/needs-attention) - #92
Merged
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughInstalls Claude Code session hooks and a hook script that writes per-session JSON status files to Changes
Sequence DiagramsequenceDiagram
participant Claude as Claude Session
participant Hook as Hook Script
participant FS as Filesystem (STATUS_DIR)
participant Main as CodeV Main
participant Preload as Preload / IPC
participant UI as Switcher UI
Claude->>Hook: Emit hook event (e.g., working/idle/AskUserQuestion/SessionEnd)
Hook->>FS: Write/modify `~/.claude/codev-status/{sessionId}.json` (atomic rename) or delete on end
FS->>Main: fs.watch detects change
Main->>Main: readAllStatuses() / merge / cleanup / persist
Main->>Preload: send `session-statuses-updated` IPC with status map
Preload->>UI: onSessionStatusesUpdated(callback) delivers update
UI->>UI: merge statuses -> update dot color/animation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Owner
Author
|
@cubic-dev-ai review this PR |
@grimmerk I have started the AI code review. It will take a few minutes to complete. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/session-status-hooks.ts">
<violation number="1" location="src/session-status-hooks.ts:47">
P2: The `$CWD` value is interpolated into JSON without escaping special characters. A working directory containing a `"` or `\` will produce malformed JSON, causing `readAllStatuses` to silently drop that session's status.
Consider piping through a minimal escape or using a tool that handles JSON encoding (e.g., `jq` or `printf '%s' "$CWD" | sed 's/\\/\\\\\\\\/g; s/"/\\\\"/g'`).</violation>
<violation number="2" location="src/session-status-hooks.ts:163">
P2: `SessionStatus` includes `'active'` (never produced) and omits `'unknown'` (produced by the script's `*` fallback case). The `as SessionStatus` cast in `readAllStatuses` hides the mismatch at runtime. Either add `'unknown'` to the type and remove `'active'`, or update the script's fallback to produce a value that's in the type.</violation>
</file>
<file name="src/main.ts">
<violation number="1" location="src/main.ts:1794">
P1: Wrap the body of `initSessionStatusHooks` in a try/catch to prevent an unhandled promise rejection from crashing the app on startup. `installHooks()` and `watchStatusDir()` both perform filesystem operations that can throw (e.g., permission denied on `~/.claude/settings.json`).</violation>
</file>
<file name="docs/session-status-hooks-design.md">
<violation number="1" location="docs/session-status-hooks-design.md:97">
P2: The `echo … > file` write is non-atomic — shell redirection truncates the file first, so `fs.watch` can trigger a read of an empty or partial file. Write to a temp file and `mv` it into place (rename is atomic on the same filesystem). The fix suggestion above addresses this as well.
Also consider sanitizing `SESSION_ID` (e.g., reject anything that isn't `[a-zA-Z0-9_-]`) before using it as a filename, to prevent path-traversal if the value is ever unexpected.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
stop_reason can be null in some JSONL entries. Instead, treat any last assistant message without pending AskUserQuestion as idle. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
grimmerk
force-pushed
the
feat/session-status-hooks
branch
from
April 4, 2026 10:57
799ae63 to
c31c4d1
Compare
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
grimmerk
marked this pull request as ready for review
April 4, 2026 11:01
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
src/main.ts (1)
1795-1808: Consider extracting duplicate watcher setup logic.The watcher callback logic in
initSessionStatusHooks(Lines 1801-1805) andset-session-status-hooks-enabled(Lines 1819-1823) is identical. Consider extracting to a helper function for maintainability.Proposed refactor
+const startStatusWatcher = () => { + if (!statusWatcherCleanup) { + statusWatcherCleanup = watchStatusDir((statuses) => { + const obj: Record<string, SessionStatus> = {}; + statuses.forEach((v, k) => { obj[k] = v; }); + switcherWindow?.webContents.send('session-statuses-updated', obj); + }); + } +}; + const initSessionStatusHooks = async () => { const enabled = (await settings.get('session-status-hooks')) !== false; if (enabled) { installHooks(); - if (!statusWatcherCleanup) { - statusWatcherCleanup = watchStatusDir((statuses) => { - const obj: Record<string, SessionStatus> = {}; - statuses.forEach((v, k) => { obj[k] = v; }); - switcherWindow?.webContents.send('session-statuses-updated', obj); - }); - } + startStatusWatcher(); } };Also applies to: 1814-1832
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.ts` around lines 1795 - 1808, Duplicate watcher callback logic used in initSessionStatusHooks and the 'set-session-status-hooks-enabled' handler should be extracted into a single helper: create a named function (e.g., sessionStatusWatcherCallback or buildStatusWatcher) that takes the statuses Map/Array and performs the same conversion to Record<string, SessionStatus> and calls switcherWindow?.webContents.send('session-statuses-updated', obj); then replace the inline arrow functions passed to watchStatusDir in both initSessionStatusHooks and the IPC handler (references: initSessionStatusHooks, statusWatcherCleanup, watchStatusDir, switcherWindow.webContents.send) with this helper to remove duplication and keep behavior identical.docs/session-status-hooks-design.md (1)
18-28: Optional: Add language specifiers to fenced code blocks.Static analysis flagged these code blocks as missing language specifiers. For diagram/pseudocode blocks,
textorplaintextwould satisfy the linter while maintaining readability.Also applies to: 32-41, 116-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/session-status-hooks-design.md` around lines 18 - 28, Add a language specifier to the fenced code blocks that are currently untagged (for example the block starting with "Claude Code session → hook fires (Stop/PermissionRequest/UserPromptSubmit/etc.) → runs ~/.claude/codev-status-hook.sh → writes ~/.claude/codev-status/{sessionId}.json" and the similar pseudocode blocks later) by changing ``` to ```text (or ```plaintext) so the linter stops flagging them; update every untagged fenced block in this document (including the blocks around the other pseudocode/diagram sections) to use a language specifier.src/switcher-ui.tsx (1)
974-988: Status colors differ from design doc — verify intended values.The code uses
#E8956A(orange) for "working" status, but the design doc (Line 11) specifies Purple#CE93D8for "Working". The PR description matches this code. If the code is correct, updatedocs/session-status-hooks-design.mdLine 11 to match.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/switcher-ui.tsx` around lines 974 - 988, The status dot color for "working" in the session rendering block is set to `#E8956A` but the design doc specifies purple `#CE93D8`; update the JSX conditional that reads sessionStatuses[session.sessionId] in switcher-ui.tsx so the 'working' branch uses '#CE93D8' (leave the existing animation logic such as 'statusPulse' unchanged), or if the current orange is intentionally correct then update the design doc entry for "Working" to match `#E8956A`; ensure whichever you change keeps the mapping for status -> color coherent with sessionStatuses and the rendered span.src/session-status-hooks.ts (4)
213-219: Movechild_processimport to top level.Using
require()inside the function works but is unconventional. Top-level imports are cleaner and allow the bundler/type-checker to validate the module.♻️ Proposed fix
Add to top-level imports:
import { exec } from 'child_process';Then update the function:
export const scanInitialStatuses = async ( activeSessions: { sessionId: string; project: string }[], ): Promise<Map<string, SessionStatus>> => { - const { exec } = require('child_process'); const execPromise = (cmd: string): Promise<string> =>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/session-status-hooks.ts` around lines 213 - 219, Move the child_process require to a top-level import and update execPromise to use that import: add "import { exec } from 'child_process';" at the file top and remove the inline const { exec } = require('child_process'); inside the function, keeping execPromise signature and implementation otherwise (use the imported exec, keep the Promise wrapper, options and callback handling as-is).
8-24: Import order and trailing comma violations.Per coding guidelines:
- Imports should be organized alphabetically:
osshould come beforepath.- Arrays should use trailing commas.
♻️ Proposed fix
import * as fs from 'fs'; -import * as path from 'path'; import * as os from 'os'; +import * as path from 'path';const HOOK_EVENTS = [ 'Stop', 'UserPromptSubmit', 'PermissionRequest', 'SubagentStart', 'SessionEnd', +]; -];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/session-status-hooks.ts` around lines 8 - 24, Reorder the top-level imports in session-status-hooks.ts so they are alphabetical (move the 'os' import before 'path' and ensure the rest remain sorted), and add a trailing comma to the HOOK_EVENTS array definition (after 'SessionEnd') to satisfy the trailing-comma rule; update the import block and the HOOK_EVENTS const accordingly (symbols: imports of fs, os, path and the HOOK_EVENTS constant).
188-200: Consider debouncing rapid fs.watch events.
fs.watchcan fire multiple events in quick succession (especially on macOS). Without debouncing,onChangemay be called repeatedly, potentially causing unnecessary re-renders in the UI.♻️ Proposed fix with simple debounce
export const watchStatusDir = ( onChange: (statuses: Map<string, SessionStatus>) => void, ): (() => void) => { fs.mkdirSync(STATUS_DIR, { recursive: true }); + let debounceTimer: NodeJS.Timeout | null = null; const watcher = fs.watch(STATUS_DIR, { persistent: false }, () => { - onChange(readAllStatuses()); + if (debounceTimer) clearTimeout(debounceTimer); + debounceTimer = setTimeout(() => { + onChange(readAllStatuses()); + }, 50); }); return () => { + if (debounceTimer) clearTimeout(debounceTimer); watcher.close(); }; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/session-status-hooks.ts` around lines 188 - 200, fs.watch in watchStatusDir can emit rapid duplicate events; wrap the callback so multiple quick events coalesce into a single onChange/readAllStatuses call by adding a debounce timer (e.g., 50–100ms) inside the watcher callback, reset the timer on each event, and call onChange(readAllStatuses()) when the timer fires; also ensure the returned cleanup function closes watcher and clears the debounce timer to avoid leaks (refer to watchStatusDir, fs.watch, onChange, readAllStatuses, and watcher).
84-88: Missing trailing comma in hook configuration object.Per coding guidelines, objects should use trailing commas.
♻️ Proposed fix
settings.hooks[event].push({ matcher: '', - hooks: [{ type: 'command', command: HOOK_SCRIPT_PATH, timeout: 5 }], + hooks: [{ type: 'command', command: HOOK_SCRIPT_PATH, timeout: 5, }], });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/session-status-hooks.ts` around lines 84 - 88, The pushed hook object is missing trailing commas per style rules: update the object passed to settings.hooks[event].push so both the outer object property list and the inner hook object use trailing commas — ensure matcher: '' and hooks: [...] end with a comma, and inside hooks the { type: 'command', command: HOOK_SCRIPT_PATH, timeout: 5 } entry also has a trailing comma; leave modified = true unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/session-status-hooks-design.md`:
- Around line 9-15: Update the docs table to match the UI implementation: change
the "Working" row dot color to Orange `#E8956A` and add "2s pulse" in the
Animation column, keep "Idle" as Green `#66BB6A` with "static", set "Needs
attention" to Orange `#FFA726` with "1s blink", and leave "Active (unknown)" as
Purple `#CE93D8` static; reference the UI implementation in src/switcher-ui.tsx
(see the working-status dot and animation around the component that renders
session status) to ensure the values and wording match the actual CSS/animation
used.
In `@src/main.ts`:
- Around line 1842-1849: Move the readClaudeSessions(500) call out of the .map
so it is executed once: before building sessionsWithoutStatus, call const
sessions = readClaudeSessions(500) and then use that local sessions array inside
the .map callback; update the sessionsWithoutStatus computation (which filters
activeMap.entries(), checks !obj[sessionId], and maps to find the session by
sessionId) to reference the preloaded sessions variable so you avoid repeated
I/O in sessionsWithoutStatus, activeMap, obj, and the inner session lookup.
In `@src/session-status-hooks.ts`:
- Line 47: The shell write command currently writes CWD raw into JSON and must
be changed to emit properly escaped JSON; update the echo line that writes to
"$STATUS_DIR/$SESSION_ID.json" (the line using STATUS, CWD, STATUS_DIR,
SESSION_ID) to produce JSON with CWD safely encoded—preferably by using jq
(e.g., jq -n --arg status "$STATUS" --arg cwd "$CWD" ...) or, if jq isn't
available, invoke a small JSON encoder (python -c or node -e) or explicitly
escape CWD before embedding—so the written file is always valid JSON and
readAllStatuses() will not fail silently.
- Line 237: The code interpolates jsonlPath/sessionId into a shell string for
execPromise (const tail = await execPromise(`tail -n 50 "${jsonlPath}"`)), which
risks shell injection; validate sessionId as a UUID (or otherwise
validate/normalize jsonlPath) before use and replace the shell-based execPromise
call with a shell-avoiding variant (e.g., create an execFilePromise wrapper that
calls execFile('tail', ['-n','50', jsonlPath']) or use spawn) so the command
arguments are passed safely; update any callers that use execPromise for tailing
to use the new execFilePromise and include a short timeout and proper error
handling.
---
Nitpick comments:
In `@docs/session-status-hooks-design.md`:
- Around line 18-28: Add a language specifier to the fenced code blocks that are
currently untagged (for example the block starting with "Claude Code session →
hook fires (Stop/PermissionRequest/UserPromptSubmit/etc.) → runs
~/.claude/codev-status-hook.sh → writes ~/.claude/codev-status/{sessionId}.json"
and the similar pseudocode blocks later) by changing ``` to ```text (or
```plaintext) so the linter stops flagging them; update every untagged fenced
block in this document (including the blocks around the other pseudocode/diagram
sections) to use a language specifier.
In `@src/main.ts`:
- Around line 1795-1808: Duplicate watcher callback logic used in
initSessionStatusHooks and the 'set-session-status-hooks-enabled' handler should
be extracted into a single helper: create a named function (e.g.,
sessionStatusWatcherCallback or buildStatusWatcher) that takes the statuses
Map/Array and performs the same conversion to Record<string, SessionStatus> and
calls switcherWindow?.webContents.send('session-statuses-updated', obj); then
replace the inline arrow functions passed to watchStatusDir in both
initSessionStatusHooks and the IPC handler (references: initSessionStatusHooks,
statusWatcherCleanup, watchStatusDir, switcherWindow.webContents.send) with this
helper to remove duplication and keep behavior identical.
In `@src/session-status-hooks.ts`:
- Around line 213-219: Move the child_process require to a top-level import and
update execPromise to use that import: add "import { exec } from
'child_process';" at the file top and remove the inline const { exec } =
require('child_process'); inside the function, keeping execPromise signature and
implementation otherwise (use the imported exec, keep the Promise wrapper,
options and callback handling as-is).
- Around line 8-24: Reorder the top-level imports in session-status-hooks.ts so
they are alphabetical (move the 'os' import before 'path' and ensure the rest
remain sorted), and add a trailing comma to the HOOK_EVENTS array definition
(after 'SessionEnd') to satisfy the trailing-comma rule; update the import block
and the HOOK_EVENTS const accordingly (symbols: imports of fs, os, path and the
HOOK_EVENTS constant).
- Around line 188-200: fs.watch in watchStatusDir can emit rapid duplicate
events; wrap the callback so multiple quick events coalesce into a single
onChange/readAllStatuses call by adding a debounce timer (e.g., 50–100ms) inside
the watcher callback, reset the timer on each event, and call
onChange(readAllStatuses()) when the timer fires; also ensure the returned
cleanup function closes watcher and clears the debounce timer to avoid leaks
(refer to watchStatusDir, fs.watch, onChange, readAllStatuses, and watcher).
- Around line 84-88: The pushed hook object is missing trailing commas per style
rules: update the object passed to settings.hooks[event].push so both the outer
object property list and the inner hook object use trailing commas — ensure
matcher: '' and hooks: [...] end with a comma, and inside hooks the { type:
'command', command: HOOK_SCRIPT_PATH, timeout: 5 } entry also has a trailing
comma; leave modified = true unchanged.
In `@src/switcher-ui.tsx`:
- Around line 974-988: The status dot color for "working" in the session
rendering block is set to `#E8956A` but the design doc specifies purple `#CE93D8`;
update the JSX conditional that reads sessionStatuses[session.sessionId] in
switcher-ui.tsx so the 'working' branch uses '#CE93D8' (leave the existing
animation logic such as 'statusPulse' unchanged), or if the current orange is
intentionally correct then update the design doc entry for "Working" to match
`#E8956A`; ensure whichever you change keeps the mapping for status -> color
coherent with sessionStatuses and the rendered span.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 616e225d-3445-4411-a3e7-db4753d09bb7
📒 Files selected for processing (9)
CHANGELOG.mddocs/session-status-hooks-design.mdpackage.jsonsrc/electron-api.d.tssrc/main.tssrc/popup.tsxsrc/preload.tssrc/session-status-hooks.tssrc/switcher-ui.tsx
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/session-status-hooks.ts">
<violation number="1" location="src/session-status-hooks.ts:237">
P1: Use `execFile` instead of `exec` to avoid shell injection. The `jsonlPath` includes unsanitized `session.sessionId`, and `exec` spawns a shell that interprets metacharacters. `execFile` bypasses the shell entirely.</violation>
</file>
<file name="src/main.ts">
<violation number="1" location="src/main.ts:1845">
P1: `readClaudeSessions(500)` is called inside the `.map()` loop, re-reading session JSONL data for every active session without a status file. Hoist it before the loop.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…c write, CWD escape, execFile Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/session-status-hooks-design.md (1)
11-11:⚠️ Potential issue | 🟡 MinorDocumentation: "Working" status color doesn't match implementation.
Line 11 specifies Purple
#CE93D8for "Working" status, but the implementation uses Orange#E8956Awith a 2s pulse animation. The table should also include an Animation column to document the visual behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/session-status-hooks-design.md` at line 11, Update the documentation row for the "Working" status to match the implementation: change the color value from Purple `#CE93D8` to Orange `#E8956A`, add a new "Animation" column to the status table and document the visual behavior as a 2s pulse for the "Working" row, and ensure the existing hook column still references `UserPromptSubmit`; make the table headers and formatting consistent with this new column.
🧹 Nitpick comments (2)
docs/session-status-hooks-design.md (2)
18-18: Specify language for fenced code blocks.Three code blocks are missing language specifiers (lines 18, 32, 116). While these are pseudocode/diagrams rather than executable code, specifying
textorplaintextimproves rendering consistency and satisfies linting rules.📝 Suggested fix
-``` +```text Claude Code session → hook fires (Stop/PermissionRequest/UserPromptSubmit/etc.)Apply the same change to the blocks at lines 32 and 116.
As per static analysis, fenced code blocks should have a language specified (markdownlint MD040).
Also applies to: 32-32, 116-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/session-status-hooks-design.md` at line 18, Three fenced code blocks in the document are missing language specifiers (the pseudocode/diagram blocks at the three locations noted); update each block (the one containing "Claude Code session → hook fires (Stop/PermissionRequest/UserPromptSubmit/etc.)" and the other two similar pseudocode blocks) to include a language token such as text or plaintext (e.g., replace ``` with ```text) so the markdown linter MD040 is satisfied and rendering is consistent.
231-231: Replace local path with public URL or remove.The reference
~/git/cmux/CLI/cmux.swiftis a local filesystem path that won't work for other developers. Either replace it with a public GitHub URL or remove the reference if it's not essential.🔗 Suggested fix
-- [cmux claude-hook](~/git/cmux/CLI/cmux.swift) — cmux's hook-based session monitoring +- [cmux claude-hook](https://github.com/your-org/cmux/blob/main/CLI/cmux.swift) — cmux's hook-based session monitoringOr remove the line if the reference is not publicly available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/session-status-hooks-design.md` at line 231, The markdown entry linking "cmux claude-hook" uses a local filesystem URL (`~/git/cmux/CLI/cmux.swift`) which is not accessible to others; update the link target in the line `[cmux claude-hook](~/git/cmux/CLI/cmux.swift)` to a public repository URL (e.g., the GitHub HTTPS link to the cmux/CLI/cmux.swift file) or remove the entire line if no public source exists, ensuring the visible link text "cmux claude-hook" remains meaningful or is deleted accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/session-status-hooks-design.md`:
- Around line 114-123: The merge strategy that reads/modifies/writes
settings.json using fs.readFileSync()/fs.writeFileSync() (see the session status
hook install routine, e.g., ensureSessionStatusHooks / installSessionHooks in
src/session-status-hooks.ts) is racy and can corrupt or lose concurrent edits;
either document this limitation in the design doc as an accepted constraint for
single-instance usage, or change the implementation to perform atomic updates by
writing to a temporary file and renaming (or use an existing atomic helper like
write-file-atomic or the electron-settings atomic API) when persisting
settings.json and ensure you replace the direct fs.writeFileSync call with that
atomic write path and add simple retry/lock logic around read-modify to minimize
race windows.
---
Duplicate comments:
In `@docs/session-status-hooks-design.md`:
- Line 11: Update the documentation row for the "Working" status to match the
implementation: change the color value from Purple `#CE93D8` to Orange
`#E8956A`, add a new "Animation" column to the status table and document the
visual behavior as a 2s pulse for the "Working" row, and ensure the existing
hook column still references `UserPromptSubmit`; make the table headers and
formatting consistent with this new column.
---
Nitpick comments:
In `@docs/session-status-hooks-design.md`:
- Line 18: Three fenced code blocks in the document are missing language
specifiers (the pseudocode/diagram blocks at the three locations noted); update
each block (the one containing "Claude Code session → hook fires
(Stop/PermissionRequest/UserPromptSubmit/etc.)" and the other two similar
pseudocode blocks) to include a language token such as text or plaintext (e.g.,
replace ``` with ```text) so the markdown linter MD040 is satisfied and
rendering is consistent.
- Line 231: The markdown entry linking "cmux claude-hook" uses a local
filesystem URL (`~/git/cmux/CLI/cmux.swift`) which is not accessible to others;
update the link target in the line `[cmux
claude-hook](~/git/cmux/CLI/cmux.swift)` to a public repository URL (e.g., the
GitHub HTTPS link to the cmux/CLI/cmux.swift file) or remove the entire line if
no public source exists, ensuring the visible link text "cmux claude-hook"
remains meaningful or is deleted accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a9460e00-6d00-4cec-9b88-a24bc8858acc
📒 Files selected for processing (1)
docs/session-status-hooks-design.md
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/session-status-hooks.ts (1)
66-99:⚠️ Potential issue | 🔴 CriticalMake
~/.claude/settings.jsonupdates fail-safe before release.If
JSON.parsefails at Line 70,installHooks()falls back to{}and Line 98 writes that back with only CodeV hooks, which can wipe unrelated Claude settings. Both install/remove also do raw read-modify-write withwriteFileSync, so concurrent/manual edits or a crash can still corrupt the file, andremoveHooks()deletes the script before the JSON update succeeds. Fail closed on parse errors and persist the settings file via temp-file+rename before deleting the script.Also applies to: 105-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/session-status-hooks.ts` around lines 66 - 99, installHooks()/removeHooks() currently fall back to {} on JSON.parse errors and do raw writeFileSync which can wipe unrelated settings or race with concurrent edits; change both functions to fail-closed on parse errors (if SETTINGS_PATH exists and JSON.parse throws, abort and return an error rather than overwrite), perform updates by writing to a temp file and then atomically renaming into SETTINGS_PATH (temp-file+rename) to avoid partial/corrupt writes, and ensure HOOK_SCRIPT_PATH is not deleted in removeHooks() until the settings JSON update succeeds; use the unique symbols SETTINGS_PATH, HOOK_SCRIPT_PATH, HOOK_MARKER, HOOK_EVENTS, installHooks, and removeHooks to locate and adjust the logic.
🧹 Nitpick comments (1)
src/main.ts (1)
1839-1841: Propagate theSessionStatusunion through the preload types.This handler returns
Record<string, SessionStatus>, butsrc/electron-api.d.ts:42-48still exposesPromise<Record<string, string | null>>, andsrc/switcher-ui.tsx:509-517consumesRecord<string, string>. That drops the literal union and weakens exhaustiveness checks for new status values. As per coding guidelines,**/*.{ts,tsx}: Use TypeScript for all components with strict typing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.ts` around lines 1839 - 1841, The code currently builds obj from readAllStatuses but the preload/type declaration and consumers still use loose string types; update the types so SessionStatus is propagated end-to-end: change the preload/export signature that currently exposes Promise<Record<string, string | null>> in electron-api.d.ts to Promise<Record<string, SessionStatus>> (and ensure the preload implementation returns that shape from readAllStatuses), then update consumers such as the switcher UI (the code around the handling in switcher-ui.tsx lines ~509-517) to accept and use Record<string, SessionStatus> instead of Record<string, string> (and adjust any null-handling or string-only assumptions accordingly) so the literal union and exhaustiveness checks are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/session-status-hooks-design.md`:
- Around line 18-28: The markdown contains unlabeled fenced code blocks (the
pseudocode blocks starting with "Claude Code session" / "CodeV (main process)"
and the other similar blocks referenced in your comment) which trigger
markdownlint MD040; add a language identifier (use "text") after the opening
triple backticks for each unlabeled fence in docs/session-status-hooks-design.md
(including the blocks around the other sections you noted) so they become
```text and satisfy the linter while preserving the plain-text pseudocode.
In `@src/main.ts`:
- Around line 1802-1805: watchStatusDir currently sends only the current on-disk
snapshot so removed status files never get propagated; change the watcher
callback that builds obj (the one that calls
switcherWindow?.webContents.send('session-statuses-updated', obj)) to diff
against a stored previousStatuses map: for any key present in previousStatuses
but missing in the new statuses, set obj[thatKey] = null (so the renderer can
clear state), then update previousStatuses = new snapshot; apply the same
diffing/null-emission logic to the other watcher block that also calls
'session-statuses-updated'.
- Around line 1838-1862: The handler for 'get-session-statuses' ignores the user
toggle and still reads status files and scans active sessions; update the
ipcMain.handle('get-session-statuses') entrypoint (the get-session-statuses
handler) to check the session-status-hooks setting first and immediately return
an empty object if the feature is disabled, avoiding calls to readAllStatuses(),
detectActiveSessions(), readClaudeSessions(), and scanInitialStatuses(); keep
the existing logic intact for the enabled path so behavior is unchanged when the
toggle is on.
---
Duplicate comments:
In `@src/session-status-hooks.ts`:
- Around line 66-99: installHooks()/removeHooks() currently fall back to {} on
JSON.parse errors and do raw writeFileSync which can wipe unrelated settings or
race with concurrent edits; change both functions to fail-closed on parse errors
(if SETTINGS_PATH exists and JSON.parse throws, abort and return an error rather
than overwrite), perform updates by writing to a temp file and then atomically
renaming into SETTINGS_PATH (temp-file+rename) to avoid partial/corrupt writes,
and ensure HOOK_SCRIPT_PATH is not deleted in removeHooks() until the settings
JSON update succeeds; use the unique symbols SETTINGS_PATH, HOOK_SCRIPT_PATH,
HOOK_MARKER, HOOK_EVENTS, installHooks, and removeHooks to locate and adjust the
logic.
---
Nitpick comments:
In `@src/main.ts`:
- Around line 1839-1841: The code currently builds obj from readAllStatuses but
the preload/type declaration and consumers still use loose string types; update
the types so SessionStatus is propagated end-to-end: change the preload/export
signature that currently exposes Promise<Record<string, string | null>> in
electron-api.d.ts to Promise<Record<string, SessionStatus>> (and ensure the
preload implementation returns that shape from readAllStatuses), then update
consumers such as the switcher UI (the code around the handling in
switcher-ui.tsx lines ~509-517) to accept and use Record<string, SessionStatus>
instead of Record<string, string> (and adjust any null-handling or string-only
assumptions accordingly) so the literal union and exhaustiveness checks are
preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 173b2d41-faf7-4362-8fcf-a4c2de8e4e9f
📒 Files selected for processing (3)
docs/session-status-hooks-design.mdsrc/main.tssrc/session-status-hooks.ts
…iles Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main.ts`:
- Around line 1819-1836: The IPC handler for 'set-session-status-hooks-enabled'
tears down the watcher (statusWatcherCleanup) and calls removeHooks() when
disabling, but never sends a session-statuses-updated message so the renderer
can clear stale UI; update the handler (inside the ipcMain.on callback) to
explicitly send an empty snapshot via
switcherWindow?.webContents.send('session-statuses-updated', {}) immediately
after calling statusWatcherCleanup() / removeHooks() (when enabled === false) so
turning the toggle OFF deterministically clears statuses in the renderer; keep
the existing watchStatusDir/statusWatcherCleanup logic intact for the enabled
path.
In `@src/session-status-hooks.ts`:
- Around line 66-74: The current try/catch silently swallows JSON parse errors
and sets settings = {}, which leads subsequent code that writes settings (the
save/persist call that writes to SETTINGS_PATH) to overwrite the existing
corrupted/unreadable file; instead, detect and propagate the parse error so we
do not overwrite the file—i.e., in the read block around SETTINGS_PATH, catch
the error, log or rethrow/exit (abort the install) rather than assigning
settings = {}, and ensure the code path that writes settings (the save/persist
operation that writes SETTINGS_PATH) only runs when reading succeeded or when
creating a new file, not after a parse failure; update the catch to throw or
call process.exit(1) with an error message referencing SETTINGS_PATH and do not
perform the write when an error was detected.
In `@src/switcher-ui.tsx`:
- Around line 542-545: The focus-refresh handler currently merges the snapshot
from window.electronAPI.getSessionStatuses() into the existing state (using
setSessionStatuses(prev => ({ ...prev, ...statuses }))) which prevents removal
of session IDs that no longer exist; change the handler to replace the map
instead (call setSessionStatuses with the returned statuses directly or
normalize to an empty object when undefined) so it mirrors the watcher
callback's behavior and fully reconciles the source-of-truth returned by
getSessionStatuses().
- Around line 510-517: The effect registers IPC listeners like
window.electronAPI.getSessionStatuses(),
window.electronAPI.onSessionStatusesUpdated, and other channels
(onSwitchToTerminal, onCheckTerminalAndHide, onFocusWindow,
onWorkingFolderIterated, onXWinNotFound, onFolderSelected) but the cleanup only
removes DOM listeners; fix by capturing each listener callback reference when
you call onSessionStatusesUpdated (and the other on* registrations) and then in
the effect's cleanup call window.electronAPI.removeListener(channel, handler)
(or ipcRenderer.removeListener) for each corresponding channel so handlers are
removed on unmount/remount, mirroring the pattern used in ai-assistant-ui.tsx
for removeListener.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0f6a9159-ae76-44b2-ab86-1d2582a0a7d5
📒 Files selected for processing (3)
src/main.tssrc/session-status-hooks.tssrc/switcher-ui.tsx
…ace on focus Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/session-status-hooks.ts (1)
66-74:⚠️ Potential issue | 🟠 MajorDo not rewrite
settings.jsonafter a read or parse failure.This
catchfalls back to{}, and the later write persists that fallback back to Claude's settings file. A malformed or unreadablesettings.jsonwould therefore lose unrelated user settings as soon as installation continues. Abort instead of writing the fallback object.Also applies to: 97-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/session-status-hooks.ts` around lines 66 - 74, The current try/catch replaces a failed read/parse of SETTINGS_PATH with an empty object and later code writes that back, potentially clobbering user settings; instead, in the catch block for the fs.readFileSync/JSON.parse attempt set a flag (e.g., settingsLoaded = false) or rethrow/exit so you do not proceed to write back a fallback, and only allow any code that writes the settings file to run when settingsLoaded is true (or after a successful parse). Update the error handling around SETTINGS_PATH, fs.existsSync, fs.readFileSync and JSON.parse so a malformed/unreadable settings.json causes abort/skip-write rather than silently using {} and persisting it; apply the same change to the other identical read block (the 97-99 occurrence).
🧹 Nitpick comments (1)
src/session-status-hooks.ts (1)
168-183: Keep persisted statuses type-safe at the boundary.
readAllStatuses()casts arbitrary JSON toSessionStatus, whilewriteStatusFile()accepts anystring. That bypasses the compiler and lets unsupported values leak into the renderer. Type the writer asExclude<SessionStatus, null>and validate parsed JSON before inserting it into the map.As per coding guidelines, Use TypeScript for all components with strict typing.
Also applies to: 319-320
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/session-status-hooks.ts` around lines 168 - 183, The persisted status handling is not type-safe: update writeStatusFile to accept a strictly typed status (use Exclude<SessionStatus, null>) instead of any string, and update readAllStatuses to validate parsed JSON before inserting into the Map (check that the parsed value is one of the allowed literals 'working' | 'idle' | 'needs-attention' | 'unknown' before casting) so unsupported values are rejected; adjust the signatures and the JSON-read branch in readAllStatuses (the code that calls JSON.parse and statuses.set) to perform this validation and only set the Map when the value matches the SessionStatus union.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/session-status-hooks.ts`:
- Around line 84-85: The current detection in hasOurHook uses
entry.hooks?.some((h) => h.command?.includes(HOOK_MARKER)) which can match
arbitrary commands; change the predicate to match the exact managed hook
path/marker (compare h.command === HOOK_SCRIPT_PATH or use an exact equality to
the unambiguous ownership marker constant instead of includes(HOOK_MARKER)) so
only CodeV-managed hooks are detected; apply the same exact-match fix to the
other occurrences referenced (the checks around the blocks at the other two
locations).
---
Duplicate comments:
In `@src/session-status-hooks.ts`:
- Around line 66-74: The current try/catch replaces a failed read/parse of
SETTINGS_PATH with an empty object and later code writes that back, potentially
clobbering user settings; instead, in the catch block for the
fs.readFileSync/JSON.parse attempt set a flag (e.g., settingsLoaded = false) or
rethrow/exit so you do not proceed to write back a fallback, and only allow any
code that writes the settings file to run when settingsLoaded is true (or after
a successful parse). Update the error handling around SETTINGS_PATH,
fs.existsSync, fs.readFileSync and JSON.parse so a malformed/unreadable
settings.json causes abort/skip-write rather than silently using {} and
persisting it; apply the same change to the other identical read block (the
97-99 occurrence).
---
Nitpick comments:
In `@src/session-status-hooks.ts`:
- Around line 168-183: The persisted status handling is not type-safe: update
writeStatusFile to accept a strictly typed status (use Exclude<SessionStatus,
null>) instead of any string, and update readAllStatuses to validate parsed JSON
before inserting into the Map (check that the parsed value is one of the allowed
literals 'working' | 'idle' | 'needs-attention' | 'unknown' before casting) so
unsupported values are rejected; adjust the signatures and the JSON-read branch
in readAllStatuses (the code that calls JSON.parse and statuses.set) to perform
this validation and only set the Map when the value matches the SessionStatus
union.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 42c0b0aa-8522-4497-a12f-009c0419cbd0
📒 Files selected for processing (2)
src/main.tssrc/session-status-hooks.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 4, 2026
18 tasks
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Session status indicators using Claude Code hooks — colored dots show whether each session is working, idle, or needs attention.
Status indicators
#E8956AUserPromptSubmit/SubagentStarthook#66BB6AStophook#FFA726PermissionRequesthook#CE93D8How it works
~/.claude/settings.json(merges, never overwrites existing hooks)~/.claude/codev-status-hook.sh) writes status to~/.claude/codev-status/{sessionId}.jsonfs.watchfor real-time updatesSettings
Design decisions
sessionIdmapping: direct 1:1, avoids same-cwd ambiguity issues from detection layerstop_reasonunreliable: JSONL entries can have nullstop_reason— detect idle by finding last assistant message without pending tool use insteadcodev-status-hook, append if not foundRelationship with
detectActiveSessionsStatus hooks are an additional layer. Detection tells you which sessions are running; hooks tell you what state they're in.
detectActiveSessions()getSessionStatuses()+ JSONL scanfs.watchoncodev-status/detectActiveSessions()has 5s cache — overlapping calls hit cache (~0ms).Hook events comparison with claude-control
Also in this PR
grep -v "node"removed — npm-installed Claude Code now detected (fix: legacy fallback grep -v node excludes npm-installed Claude #95)Complexity
All operations are O(n) — no O(n²). The only potential O(m×s) is
allSessions.find()per session without status file, but m ≈ 0 after first startup (scan results persisted to files) and s is cached.File accumulation prevented by: SessionEnd hook (normal exit) + cleanupStaleStatuses() on startup/focus (crash/SIGKILL).
Known limitations
?ending) not implemented yet (Phase 2)Design doc
docs/session-status-hooks-design.md
Test plan
🤖 Generated with Claude Code
🤖 On behalf of @grimmerk — generated with Claude Code