Repository navigation
Fix DebugEventLog NSFileHandle ObjC exception crash - #3034
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTwo Swift source files received updates to debug logging operations, replacing manual file-handle management with safer Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
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)
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 the current code and only fix it if needed.
Inline comments:
In `@vendor/bonsplit`:
- Line 1: The vendor/bonsplit submodule pointer was updated to commit
cffd9a66e9ede9aaedefcfd0c391f0ad49652d72 which only exists on the feature branch
fix-debug-event-log-nsfilehandle-crash and depends on manaflow-ai/bonsplit#99;
revert the pointer in vendor/bonsplit to the current bonsplit main commit
(1bfb226) or restore the previous submodule state, and do not update the pointer
to cffd9a66... until the companion PR manaflow-ai/bonsplit#99 is merged and that
commit is present on bonsplit main; once the PR is merged, update the submodule
pointer to the new main commit and re-run submodule init/update and verify CI
before merging.
🪄 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: 7f18cc92-2a5f-442d-a792-7e38b38a1962
📒 Files selected for processing (3)
Sources/GhosttyTerminalView.swiftSources/WorkspaceContentView.swiftvendor/bonsplit
| @@ -1 +1 @@ | |||
| Subproject commit 73bd9f9cfd8c41ebc6f2d43eb48d3eafa4dfec00 | |||
| Subproject commit cffd9a66e9ede9aaedefcfd0c391f0ad49652d72 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that bonsplit companion PR has been merged and commit is on main branch
echo "=== Checking companion PR status ==="
gh pr view 99 --repo manaflow-ai/bonsplit --json state,merged,mergedAt,headRefName
echo ""
echo "=== Verifying commit cffd9a66e9ede9aaedefcfd0c391f0ad49652d72 is on main branch ==="
cd vendor/bonsplit
git fetch origin main
git branch -r --contains cffd9a66e9ede9aaedefcfd0c391f0ad49652d72 | grep 'origin/main'
if [ $? -eq 0 ]; then
echo "✓ Commit is on origin/main"
else
echo "✗ WARNING: Commit is NOT on origin/main"
exit 1
fiRepository: manaflow-ai/cmux
Length of output: 1212
🏁 Script executed:
#!/bin/bash
# Verify companion PR status and commit location with corrected syntax
echo "=== Checking companion PR status ==="
gh pr view 99 --repo manaflow-ai/bonsplit --json state,mergedAt,headRefName,number 2>/dev/null || echo "Could not fetch PR info"
echo ""
echo "=== Checking if commit exists in bonsplit repo ==="
cd vendor/bonsplit
git fetch origin 2>&1 | head -20
echo ""
echo "=== Listing all branches containing or referencing the commit ==="
git branch -a -v | grep -E "(main|master|cffd9a66)" || echo "Commit not found in local branches"
echo ""
echo "=== Checking git log for the specific commit ==="
git log --oneline -1 cffd9a66e9ede9aaedefcfd0c391f0ad49652d72 2>/dev/null || echo "Commit not found in local repo history"
echo ""
echo "=== Checking current state of bonsplit submodule ==="
cd - > /dev/null
git submodule status vendor/bonsplitRepository: manaflow-ai/cmux
Length of output: 776
Do not merge until the companion bonsplit PR is merged and commit is on main branch.
The submodule pointer references commit cffd9a66e9ede9aaedefcfd0c391f0ad49652d72, which:
- Does not exist on the
mainbranch of the bonsplit repo (main is currently at1bfb226) - Exists only on the feature branch
fix-debug-event-log-nsfilehandle-crash - Depends on companion PR Fix DebugEventLog NSFileHandle ObjC exception crash bonsplit#99, which is currently OPEN and not merged
Per the submodule workflow requirement, the companion PR must be merged and the commit pushed to main in the bonsplit repository before updating the pointer in cmux. Update the submodule pointer only after manaflow-ai/bonsplit#99 is merged.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vendor/bonsplit` at line 1, The vendor/bonsplit submodule pointer was updated
to commit cffd9a66e9ede9aaedefcfd0c391f0ad49652d72 which only exists on the
feature branch fix-debug-event-log-nsfilehandle-crash and depends on
manaflow-ai/bonsplit#99; revert the pointer in vendor/bonsplit to the current
bonsplit main commit (1bfb226) or restore the previous submodule state, and do
not update the pointer to cffd9a66... until the companion PR
manaflow-ai/bonsplit#99 is merged and that commit is present on bonsplit main;
once the PR is merged, update the submodule pointer to the new main commit and
re-run submodule init/update and verify CI before merging.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryReplaces ObjC-exception-raising
Confidence Score: 4/5Safe to merge once the companion bonsplit PR is confirmed merged to main and the missing seek guard is addressed. The core crash fix is sound and consistently applied to all four documented call sites. Two concerns block a 5: the logBackground missing guard (pre-existing path not updated by this PR) and the submodule safety requirement per CLAUDE.md that needs explicit verification before landing. vendor/bonsplit (submodule merge-base check) and Sources/GhosttyTerminalView.swift line 3475 (logBackground seek guard). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[log function called] --> B{File exists at path?}
B -- No --> C[FileManager.createFile]
B -- Yes --> D[FileHandle forWritingAtPath / forWritingTo]
D --> E[defer: try? handle.close]
E --> F{try? handle.seekToEnd != nil?}
F -- throws/nil --> G[return — no write]
F -- OK / UInt64 --> H[try? handle.write contentsOf data]
H --> I[defer fires: close]
style G fill:#f96,color:#000
style H fill:#6f6,color:#000
style C fill:#adf,color:#000
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -1 +1 @@ | |||
| Subproject commit 73bd9f9cfd8c41ebc6f2d43eb48d3eafa4dfec00 | |||
| Subproject commit cffd9a66e9ede9aaedefcfd0c391f0ad49652d72 | |||
There was a problem hiding this comment.
Per the project's submodule safety policy (CLAUDE.md): the submodule commit must be reachable from origin/main in the bonsplit repo before the parent pointer is committed here. The PR description says this PR depends on bonsplit PR #99 being merged first. Before merging, please verify:
cd vendor/bonsplit && git merge-base --is-ancestor HEAD origin/mainIf that exits non-zero, the bonsplit commit isn't on main yet and will be orphaned once the feature branch is deleted.
* Fix NSFileHandle ObjC exception crash in debug logging * Guard logBackground seek to avoid writing at offset 0 on failure
This branch was successfully deployed
1 participant
Fixes #1924.
Depends on the companion bonsplit PR: manaflow-ai/bonsplit#99
Problem
DebugEventLog.log(_:)(and three mirror call sites in the outer repo) used the deprecated Objective-C-styleFileHandleAPIs:seekToEndOfFile(),write(_:), andcloseFile(). When those fail (log file unlinked, disk full, permission issue, etc.) they raiseNSFileHandleOperationException. Objective-C exceptions are not caught by Swiftdo/catch, so the exception propagates out of the Swift runtime and the uncaught-exception handler callsabort(). That is the crash reported in #1924.Fix
Switch every call site to the Swift-throwing replacements and guard appropriately:
try? handle.seekToEnd()(notseekToEndOfFile())try? handle.write(contentsOf: data)(notwrite(_:))try? handle.close()indefer(notcloseFile())guard (try? handle.seekToEnd()) != nil else { return }so we never blindly append at offset 0 when the seek throwsThese APIs throw Swift errors instead of raising ObjC exceptions, so a transient log-write failure silently drops the entry instead of crashing the app. Changes stay inside the existing
#if DEBUGguards.Why this replaces #1931
#1931 from an external contributor only touched the two outer-repo files and stalled on rebase. It did not fix the actual root crash site at
vendor/bonsplit/Sources/Bonsplit/Public/DebugEventLog.swift(thedlog(...)implementation), which is where the most frequent crash path originates.Changed call sites
vendor/bonsplitsubmodule bump —Sources/Bonsplit/Public/DebugEventLog.swift(root cause, picks up the fix from the bonsplit PR)Sources/GhosttyTerminalView.swift— three call sites:initLog,surfaceLog,sizeLogSources/WorkspaceContentView.swift— one call site:debugPanelLookupNote
Low Risk
Changes are limited to
#if DEBUGfile logging and primarily swap APIs with defensive error handling; production behavior should be unaffected.Overview
Prevents DEBUG-only logging from crashing the app by replacing deprecated Objective-C
FileHandlecalls (seekToEndOfFile(),write(_:),closeFile()) with Swift-throwing equivalents (seekToEnd(),write(contentsOf:),close()indefer) inGhosttyTerminalViewandWorkspaceContentView.Log appends now guard against seek failures (dropping the log entry rather than writing at offset 0 / triggering an uncaught ObjC exception) while preserving existing log-file creation behavior.
Reviewed by Cursor Bugbot for commit 2b65623. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Prevent a crash in debug logging caused by ObjC
NSFileHandleexceptions by switching all debug log writes to Swift-throwing APIs and guarding seek failures, including the background log writer. Fixes #1924.seekToEndOfFile()/write(_:)/closeFile()withtry? seekToEnd(),try? write(contentsOf:), anddefer { try? close() }.#if DEBUG.Sources/GhosttyTerminalView.swift(initLog, background log, surfaceLog, sizeLog) and one inSources/WorkspaceContentView.swift(debugPanelLookup).vendor/bonsplitto include theDebugEventLogfix.Written for commit 2b65623. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Chores