-
Notifications
You must be signed in to change notification settings - Fork 1
Open
Description
Summary
The updateConfirmingWorktreeRemoval method in model.go accesses m.sessionState.Sessions[session.Name] without checking if the key exists, unlike other handlers (handleKillSession, handleArchiveSession) which use getFreshSessionInfo().
Problem
Location: internal/ui/model.go:743
When a user initiates a "kill session" action on a session with a worktree:
handleKillSessionusesgetFreshSessionInfo()to check if session exists and has worktree- If yes, it opens a confirmation dialog asking "Remove worktree too?"
- User makes a choice →
updateConfirmingWorktreeRemovalhandles the response
The gap is in step 3. Between steps 1 and 3, time passes while the user reads and interacts with the dialog. During this time, another process or user could delete the session.
Current code:
sessionInfo := m.sessionState.Sessions[session.Name]
worktreePath := sessionInfo.WorktreePath
repoPath := sessionInfo.RepoPathIf the session was deleted:
sessionInfobecomes a zero-valuedomain.Session{}worktreePathandrepoPathbecome empty strings- The code continues silently, potentially calling
RemoveWorktree("", "")with unexpected behavior
Contrast with handleKillSession (line 580):
if sessionInfo, ok := m.getFreshSessionInfo(sessionName); ok && sessionInfo.WorktreePath != "" {
// Safe: we know session exists
}Why It Matters
- Silent failures are hard to debug
- Operating on stale/missing data can lead to confusing error messages
- Inconsistent patterns make code harder to maintain
Proposed Solution
Add existence check before accessing session info:
sessionInfo, ok := m.sessionState.Sessions[session.Name]
if !ok {
logging.Logger.Warn("Session no longer exists during worktree removal", "session", session.Name)
m.state = stateList
m.worktreeRemovalForm = nil
m.sessionToKill = nil
m.formRemoveWorktree = nil
return m, m.sessionList.Init()
}Related Code
handleKillSession(line 580) - correctly usesgetFreshSessionInfohandleArchiveSession(line 596) - correctly usesgetFreshSessionInfogetFreshSessionInfohelper (line 564) - loads fresh state and checks existence
Labels
- bug
- good first issue
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels