fix: prevent preview conversation interruption when switching panels by CRY0100 · Pull Request #331 · xpert-ai/xpert

CRY0100 · 2025-12-31T02:12:20Z

Add DestroyRef to track component destruction state in ChatConversationPreviewComponent
Guard all subscription callbacks with destroyed check to prevent NG0953 error
Keep preview component mounted when switching panels using CSS display control
Add previewCreated signal and showPreview computed to preserve component state
Fixes After entering information in the preview, switching to view the environment variables panel and then switching back to the preview window will cause an error. #330

Fixes #330
Problem:
After entering information in the preview, switching to view the environment variables panel and then switching back to the preview window causes an error (NG0953: Unexpected emit for destroyed 'OutputRef'). This error interrupts the ongoing conversation.
Solution:
This PR fixes the issue by:
Adding component destruction tracking in ChatConversationPreviewComponent:
Inject DestroyRef and add a #destroyed flag to track component destruction state
Guard all subscription callbacks (next, error, complete) with destroyed check to prevent operations on destroyed components
Preserving preview component state when switching panels:
Add previewCreated signal and showPreview computed property in XpertStudioPanelComponent
Use an effect to mark the preview component as created when switching to preview panel
Keep the preview component mounted in DOM but control visibility using CSS display property instead of destroying/recreating it
This ensures the conversation state is preserved when switching between panels
Changes:
apps/cloud/src/app/@shared/chat/preview/preview.component.ts: Add DestroyRef and #destroyed checks in subscription callbacks
apps/cloud/src/app/features/xpert/studio/panel/panel.component.ts: Add previewCreated signal and showPreview computed with effect
apps/cloud/src/app/features/xpert/studio/panel/panel.component.html: Change from @switch to @if with CSS display control to keep component mounted
Testing:
✅ No errors when switching panels during an active conversation
✅ Conversation continues without interruption
✅ Preview component state is preserved when switching back
Related Issue: #330

- Add DestroyRef to track component destruction state in ChatConversationPreviewComponent - Guard all subscription callbacks with destroyed check to prevent NG0953 error - Keep preview component mounted when switching panels using CSS display control - Add previewCreated signal and showPreview computed to preserve component state - Fixes xpert-ai#330

Copilot

Pull request overview

This PR addresses issue #330 by preventing the NG0953 error that occurs when switching panels during an active conversation in the preview. The solution implements two main strategies: adding component destruction tracking with manual checks in subscription callbacks, and preserving the preview component in the DOM using CSS display control instead of destroying/recreating it.

Add DestroyRef and #destroyed flag to track component destruction state
Keep preview component mounted when switching panels using CSS display control
Guard subscription callbacks with destroyed checks to prevent operations on destroyed components

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
apps/cloud/src/app/features/xpert/studio/panel/panel.component.ts	Adds previewCreated signal and showPreview computed to track and control preview component mounting state, with an effect to mark preview as created when panel switches to preview
apps/cloud/src/app/features/xpert/studio/panel/panel.component.html	Replaces @switch with @if blocks and CSS display control to keep preview component mounted while hiding/showing it based on panel selection
apps/cloud/src/app/@shared/chat/preview/preview.component.ts	Adds DestroyRef injection and #destroyed flag, with manual checks in subscription callbacks (next, error, complete) to guard against operations on destroyed components

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-01T03:11:08Z

        complete: () => {
+          // Check if component is destroyed to avoid unnecessary operations
+          if (this.#destroyed) {
+            return
+          }


The chatSubscription is manually managed but never cleaned up on component destruction. While there are checks to unsubscribe before creating a new subscription (line 300-302) and in onStop(), there's no cleanup when the component itself is destroyed.

According to the codebase convention (apps/cloud/src/app/features/chat/chat.service.socket.ts), subscriptions should use takeUntilDestroyed() operator. This would automatically unsubscribe when the component is destroyed, making the manual destroyed checks in the callbacks unnecessary and more robust.

Copilot · 2026-01-01T03:11:08Z

+        @if (sidePanel() === 'preview' && executionId()) {
+          <xpert-studio-panel-execution [id]="executionId()" class="h-full bg-components-panel-bg shadow-lg border-[0.5px] border-components-panel-border rounded-2xl overflow-y-auto overflow-x-hidden
+            w-[420px] 2xl:w-[460px]"
+            (close)="closeExecution()"
+          />


The execution panel is now only conditionally rendered when both sidePanel is 'preview' AND executionId exists. However, in the original code, the execution panel was rendered whenever executionId existed, regardless of which panel was active. This change could potentially break functionality if the execution panel needs to be visible while switching to other panels (variables or environments).

Verify that this behavior change is intentional and that the execution panel should only be visible when the preview panel is active.

Copilot · 2026-01-01T03:11:08Z

+
+    // Set flag when component is destroyed to prevent operations on destroyed component
+    this.#destroyRef.onDestroy(() => {
+      this.#destroyed = true


The DestroyRef.onDestroy() callback only sets the #destroyed flag but doesn't actually unsubscribe from the chatSubscription. If the component is destroyed while a chat request is in progress, the subscription will continue to run and attempt to update signals/state on a destroyed component, which is exactly the NG0953 error this PR aims to fix.

The manual destroyed checks in the subscription callbacks are a workaround, but a more robust solution would be to add the takeUntilDestroyed() operator to the subscription (line 303-406) and properly clean up the chatSubscription in the onDestroy callback.

Suggested change

this.#destroyed = true

this.#destroyed = true

this.chatSubscription?.unsubscribe()

Copilot · 2026-01-01T03:11:08Z

  booleanAttribute,
  Component,
  computed,
+  DestroyRef,
  effect,
  ElementRef,
  inject,


The takeUntilDestroyed operator from '@angular/core/rxjs-interop' is not imported but should be used according to the codebase convention (e.g., apps/cloud/src/app/features/chat/chat.service.socket.ts:3). Import this operator to properly clean up subscriptions when the component is destroyed.

Copilot · 2026-01-01T03:11:09Z

+  readonly showPreview = computed(() => 
+    this.sidePanel() === 'preview' || this.previewCreated()


The showPreview computed has a logic issue. Once previewCreated becomes true, showPreview will always return true regardless of the sidePanel value. This means the preview component will remain in the DOM even when sidePanel is null, which could lead to unnecessary memory usage and potentially unexpected behavior.

The correct logic should be: only show preview when sidePanel is 'preview', OR when sidePanel is not 'preview' but previewCreated is true (to preserve state when switching to other panels). Consider changing to:
this.sidePanel() === 'preview' || (this.previewCreated() && this.sidePanel() !== null)

Or more explicitly:
this.previewCreated() && (this.sidePanel() === 'preview' || this.sidePanel() === 'variables' || this.sidePanel() === 'environments')

Suggested change

readonly showPreview = computed(() =>

this.sidePanel() === 'preview' || this.previewCreated()

readonly showPreview = computed(() =>

this.sidePanel() === 'preview' || (this.previewCreated() && this.sidePanel() !== null)

Copilot · 2026-01-01T03:11:09Z

+          // Check if component is destroyed to avoid operations on destroyed component
+          if (this.#destroyed) {
+            return
+          }


There's a potential race condition with the #destroyed flag. Between the time when the check at line 325 passes and when operations like signal updates occur (e.g., line 345), the component could be destroyed. While this is unlikely in practice, using takeUntilDestroyed() operator on the subscription itself would be a more robust solution that prevents the subscription from emitting at all after component destruction, eliminating the need for manual checks.

Additionally, the codebase convention (as seen in apps/cloud/src/app/features/chat/chat.service.socket.ts) is to use takeUntilDestroyed() rather than manual destroy tracking, which would make this code more consistent with the rest of the codebase.

Copilot · 2026-01-01T03:11:09Z

                  this.output.update((state) => state + event.data)
                }
              } else if (event.type === ChatMessageTypeEnum.EVENT) {
+                // Component destroyed check is done at the beginning of next callback, emit directly here


The comment states "Component destroyed check is done at the beginning of next callback, emit directly here", but this is actually creating an inconsistency. The chatEvent is emitted without checking if the component is destroyed, while other operations are guarded. This could lead to the NG0953 error if the event emission triggers signal updates in a destroyed component.

For consistency and safety, either all emissions/operations should be guarded by the destroyed check, or the subscription should use takeUntilDestroyed() to prevent any emissions after destruction.

Suggested change

// Component destroyed check is done at the beginning of next callback, emit directly here

// Ensure component is not destroyed before emitting chat events and updating state

if (this.#destroyed) {

return

}

- Add takeUntilDestroyed() operator to chat subscription for automatic cleanup - Add manual chatSubscription cleanup in onDestroy as extra safety measure - Add destroyed check before chatEvent.emit to prevent operations on destroyed component - Fix showPreview logic to prevent preview from showing when sidePanel is null These changes address Copilot review suggestions and improve code robustness.

- Preserve takeUntilDestroyed() and chatSubscription cleanup optimizations - Preserve showPreview logic fix - Merge develop branch changes including @switch structure for sidePanel

CRY0100 temporarily deployed to production December 31, 2025 02:12 — with GitHub Actions Inactive

meta-d requested a review from Copilot January 1, 2026 03:07

Copilot started reviewing on behalf of meta-d January 1, 2026 03:07 View session

Copilot AI reviewed Jan 1, 2026

View reviewed changes

jayden added 2 commits January 7, 2026 14:46

Merge origin/develop: resolve conflicts while preserving optimizations

4f4149f

- Preserve takeUntilDestroyed() and chatSubscription cleanup optimizations - Preserve showPreview logic fix - Merge develop branch changes including @switch structure for sidePanel

CRY0100 temporarily deployed to production January 7, 2026 06:50 — with GitHub Actions Inactive

CRY0100 had a problem deploying to production January 7, 2026 06:57 — with GitHub Actions Failure

CRY0100 force-pushed the CRYfixEnv2Chat branch from 4088974 to 4f4149f Compare January 7, 2026 07:04

CRY0100 temporarily deployed to production January 7, 2026 07:04 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: prevent preview conversation interruption when switching panels#331

fix: prevent preview conversation interruption when switching panels#331
CRY0100 wants to merge 3 commits into
xpert-ai:developfrom
CRY0100:CRYfixEnv2Chat

CRY0100 commented Dec 31, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Copilot AI Jan 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	this.#destroyed = true
	this.#destroyed = true
	this.chatSubscription?.unsubscribe()

		readonly showPreview = computed(() =>
		this.sidePanel() === 'preview' \|\| this.previewCreated()

Uh oh!

Conversation

CRY0100 commented Dec 31, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants