All fixes are implemented and verified. Here's what each issue was an…#48
Merged
Conversation
…d what changed: The handler is correctly registered at HEAD ([voiceHandlers.ts:42](src/main/ipc/voiceHandlers.ts:42), wired last in [ipc/index.ts:72](src/main/ipc/index.ts:72), window created after registration — no race, no duplicate channels). The channel was added in your latest commit, and the errors came from a **stale main-process build**: Vite hot-reloads the renderer/preload, but the running main process kept the old bundle without the handler. The fresh `npm start` I ran booted cleanly with the handler registered. Related cleanup: the misleading `[WARN] voice: worker exited with code 0` after an *intentional* idle shutdown now logs as info (`expectedExit` flag in [VoiceManager.ts](src/main/managers/voice/VoiceManager.ts)); a real crash still warns. Added the `\0` rejection to the per-path loop in [attachmentHandlers.ts:56](src/main/ipc/attachmentHandlers.ts:56), matching the guards already in `fsHandlers`/`workspaceHandlers`. Note this was defense-in-depth, not an exploitable bypass — `AttachmentManager.stageOne` already rejects NUL bytes before touching the filesystem, and pasted filenames are neutralized by `sanitizeName`. That message is generated *inside* the Agent SDK when the CLI dies with a pending error result; the diagnostic means the **resumed transcript ends in a `tool_use` with no `tool_result`** (a prior run was killed mid-tool-call). Since Limboo always resumes the stored SDK session, the session failed the same way on every turn. Two fixes: - [AgentManager.ts](src/main/managers/AgentManager.ts) now detects this error in `runWithRecovery`, drops the corrupted `sdk_session_id` (new `forgetSdkSession`), logs a "starting a fresh SDK session" recovery diagnostic, and retries once — your chat history in Limboo is untouched; only the SDK-side conversation restarts. - Updated `@anthropic-ai/claude-agent-sdk` 0.3.195 → **0.3.201**, which includes upstream fixes for the "tool_use ids without tool_result blocks" resume bugs. - `npx vite build --config vite.renderer.config.mts` — built clean in 30.7s. - `npm run lint` — no findings. - Booted the app (`npm start`): main/preload/voice-worker all rebuilt, `Limboo main process ready`, workspace + search indexed, zero errors in the log; test instance shut down cleanly afterward. Sources: [electron#26495 — "No handler registered" from stale builds](electron/electron#26495), [Claude stop reasons — tool_use must be followed by tool_result](https://platform.claude.com/docs/en/build-with-claude/handling-stop-reasons), [Claude Code changelog — tool_use/resume fixes](https://claudefa.st/blog/guide/changelog).
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
The resilience improvements in this PR are correctly implemented and address real issues:
✅ Null-byte path validation (attachmentHandlers.ts) - Blocks path traversal attempts via null bytes, closing a security gap
✅ Corrupted resume self-healing (AgentManager.ts) - Automatically recovers from corrupted SDK transcripts instead of entering a crash loop
✅ Clean shutdown tracking (VoiceManager.ts) - Distinguishes expected worker exits from crashes for accurate logging
All changes are working correctly with no blocking issues.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
📝 WalkthroughWalkthroughThis PR adds three unrelated fixes: null-byte rejection in attachment path validation, corrupted SDK resume detection with automatic session-drop recovery in AgentManager, and an ChangesAttachment Path Validation
AgentManager Resume Corruption Recovery
VoiceManager Worker Exit Tracking
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AgentManager
participant SDK
participant Persistence
AgentManager->>SDK: resume session with sdk_session_id
SDK-->>AgentManager: error (unfinished tool_use)
AgentManager->>AgentManager: check resumeDropped flag
alt not yet dropped
AgentManager->>Persistence: forgetSdkSession(sessionId)
Persistence-->>AgentManager: sdk_session_id deleted
AgentManager->>AgentManager: continue retry loop
AgentManager->>SDK: start fresh session
end
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/managers/voice/VoiceManager.ts`:
- Around line 54-55: The worker lifecycle in VoiceManager still allows
ensureWorker() to reuse an existing worker after shutdown has already been
requested, which can attach new capture/TTS work to a process that is about to
exit. Update the worker state handling around expectedExit and the shutdown path
so that a pending idle shutdown marks the worker as unavailable, and ensure
ensureWorker() either reforks or refuses reuse once shutdown has been requested.
Also make sure the resulting exit is not treated as a normal expectedExit if new
work was still pending.
🪄 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: 64e74d30-89b8-4b15-9ab7-1d0d27b68096
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
src/main/ipc/attachmentHandlers.tssrc/main/managers/AgentManager.tssrc/main/managers/voice/VoiceManager.ts
Comment on lines
+54
to
+55
| /** Set when WE asked the worker to shut down, so its exit is not a crash. */ | ||
| private expectedExit = false; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files and symbols first
ast-grep outline src/main/managers/voice/VoiceManager.ts --view expanded
printf '\n--- worker outline ---\n'
ast-grep outline src/main/voice/worker.ts --view expanded
# Show the relevant sections with line numbers
printf '\n--- VoiceManager.ts relevant lines ---\n'
sed -n '390,460p' src/main/managers/voice/VoiceManager.ts
printf '\n--- VoiceManager.ts idle/dispose lines ---\n'
sed -n '630,690p' src/main/managers/voice/VoiceManager.ts
printf '\n--- worker.ts shutdown handling ---\n'
sed -n '410,450p' src/main/voice/worker.tsRepository: BotCoder254/limboo
Length of output: 10529
Guard against reusing a worker after shutdown is requested
ensureWorker() still treats the current worker as usable while an idle shutdown is already in flight, so new capture/TTS work can attach to a process that is about to exit. That exit is also counted as expectedExit, which hides the failure as a clean shutdown instead of a warning. Track a pending-shutdown state or refork before reusing the worker after shutdown has been requested.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/managers/voice/VoiceManager.ts` around lines 54 - 55, The worker
lifecycle in VoiceManager still allows ensureWorker() to reuse an existing
worker after shutdown has already been requested, which can attach new
capture/TTS work to a process that is about to exit. Update the worker state
handling around expectedExit and the shutdown path so that a pending idle
shutdown marks the worker as unavailable, and ensure ensureWorker() either
reforks or refuses reuse once shutdown has been requested. Also make sure the
resulting exit is not treated as a normal expectedExit if new work was still
pending.
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.
…d what changed:
The handler is correctly registered at HEAD (voiceHandlers.ts:42, wired last in ipc/index.ts:72, window created after registration — no race, no duplicate channels). The channel was added in your latest commit, and the errors came from a stale main-process build: Vite hot-reloads the renderer/preload, but the running main process kept the old bundle without the handler. The fresh
npm startI ran booted cleanly with the handler registered. Related cleanup: the misleading[WARN] voice: worker exited with code 0after an intentional idle shutdown now logs as info (expectedExitflag in VoiceManager.ts); a real crash still warns.Added the
\0rejection to the per-path loop in attachmentHandlers.ts:56, matching the guards already infsHandlers/workspaceHandlers. Note this was defense-in-depth, not an exploitable bypass —AttachmentManager.stageOnealready rejects NUL bytes before touching the filesystem, and pasted filenames are neutralized bysanitizeName.That message is generated inside the Agent SDK when the CLI dies with a pending error result; the diagnostic means the resumed transcript ends in a
tool_usewith notool_result(a prior run was killed mid-tool-call). Since Limboo always resumes the stored SDK session, the session failed the same way on every turn. Two fixes:AgentManager.ts now detects this error in
runWithRecovery, drops the corruptedsdk_session_id(newforgetSdkSession), logs a "starting a fresh SDK session" recovery diagnostic, and retries once — your chat history in Limboo is untouched; only the SDK-side conversation restarts.Updated
@anthropic-ai/claude-agent-sdk0.3.195 → 0.3.201, which includes upstream fixes for the "tool_use ids without tool_result blocks" resume bugs.npx vite build --config vite.renderer.config.mts— built clean in 30.7s.npm run lint— no findings.Booted the app (
npm start): main/preload/voice-worker all rebuilt,Limboo main process ready, workspace + search indexed, zero errors in the log; test instance shut down cleanly afterward.Sources: electron#26495 — "No handler registered" from stale builds, Claude stop reasons — tool_use must be followed by tool_result, Claude Code changelog — tool_use/resume fixes.
Summary
Type of change
Architectural reasoning
Verification
npm run lintpassesnpx vite build --config vite.renderer.config.mtspassesnpm start(for main/preload changes)Security checklist (if touching main process)
shell: true)contextIsolation/sandbox/ CSP / navigation lockdownDocumentation and changelog
docs/CHANGELOG.mdentry (if user-facing)Theme discipline (for UI changes)
dark:variants, no gradients)Note
Low Risk
Changes are localized recovery, logging, IPC validation, and a patch-level SDK bump; agent retry is bounded to one resume drop per run.
Overview
Fixes three reliability and hardening issues.
Agent: When a resumed Claude SDK session fails because the transcript ends in
tool_usewithout a matchingtool_result(e.g. a prior run was killed mid-tool),runWithRecoverynow detects that error pattern, clears the storedsdk_session_idvia newforgetSdkSession, logs a recovery diagnostic, and retries once with a fresh SDK session. Limboo’s own chat transcript is unchanged.@anthropic-ai/claude-agent-sdkis bumped 0.3.197 → 0.3.201 in the lockfile.Voice: Intentional worker shutdown (idle timer or
dispose) sets anexpectedExitflag so exit code 0 is logged at info instead of warn, while real crashes still warn and can trigger restart.Attachments:
attachmentAddPathsrejects paths containing NUL (\0) in the IPC handler, aligned with other path handlers.Reviewed by Cursor Bugbot for commit 8ef8a69. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit