Skip to content

Fix useAutomationChat missing onUnmounted cleanup (#1192)#1199

Open
Chris0Jeky wants to merge 1 commit into
mainfrom
fix/1192-automation-chat-cleanup
Open

Fix useAutomationChat missing onUnmounted cleanup (#1192)#1199
Chris0Jeky wants to merge 1 commit into
mainfrom
fix/1192-automation-chat-cleanup

Conversation

@Chris0Jeky
Copy link
Copy Markdown
Owner

Summary

  • Store the watch stop handle (const stopWatch = watch(...)) and call it during disposal
  • Add isDisposed flag (plain let, matching useGlobalSearch pattern) to guard all async continuations after every await in loadSessions, loadSession, loadProviderHealth, handleCreateSession, sendMessageToSession, and loadBoardOptions
  • Register onScopeDispose callback that sets isDisposed = true and calls stopWatch()
  • Add 3 new tests verifying the disposal callback is registered and that reactive state is not written after scope disposal

Closes #1192

Test plan

  • npm run typecheck passes
  • All 20 useAutomationChat.spec.ts tests pass (17 existing + 3 new disposal tests)
  • Full vitest suite passes (294 files, 3674 tests)
  • CI green

Add scope disposal guards to prevent watcher and async write leaks
after component teardown. Follows the existing isDisposed + onScopeDispose
pattern from useGlobalSearch.

- Store watch stop handle and call it in onScopeDispose
- Add isDisposed flag to guard all async continuations after await
- Add tests verifying reactive state is not written after disposal
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces scope disposal handling in the useAutomationChat composable by utilizing onScopeDispose and an isDisposed flag to guard asynchronous operations from writing to reactive state after the scope is destroyed. It also adds corresponding unit tests to verify this behavior. The review feedback correctly identifies an unguarded async continuation in onMounted that calls applyRouteBoardContext(), which could still write to reactive state after disposal and should be guarded.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +369 to +372
onScopeDispose(() => {
isDisposed = true
stopWatch()
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While isDisposed is set to true here, there is still an unguarded async continuation in onMounted (line 357):

void loadBoardOptions().then(() => {
  applyRouteBoardContext()
})

If loadBoardOptions() resolves after the scope is disposed, the .then() callback will still execute and call applyRouteBoardContext(). This function writes to the reactive refs newSessionBoardId and selectedNewSessionBoardId without checking isDisposed.

To fix this, you can add an isDisposed check at the beginning of applyRouteBoardContext() or guard the .then() callback in onMounted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pending

Development

Successfully merging this pull request may close these issues.

useAutomationChat missing onUnmounted cleanup — watcher and async writes leak after teardown

1 participant