[RoomLifeCycleManager] `_operationInProgress` flag incorrectly set

Currently, operationInProgress flag is set to false only when attach operation finishes.
It should be set false at the end of other operations like Detach, Release, and Internal operations.

Also, not sure if we really need this code snippet in place

ably-chat-js/src/core/room-lifecycle-manager.ts

Lines 130 to 134 in 533c4d0

    
           // This shouldn't be the case except in testing, but if we're already attached, then we should consider 
        
           // ourselves not in the middle of an operation and thus consider channel events. 
        
           if (this._lifecycle.status !== RoomStatus.Attached) { 
        
             this._operationInProgress = true; 
        
           }

This sets operationInProgress to true when the room has just initialized and not performing any operation.

┆Issue is synchronized with this Jira Task by Unito

Also, I feel operationInProgress should be set to false at the end of mtx promise, not in between right?
Like in case of Attach op, it's set in between,

ably-chat-js/src/core/room-lifecycle-manager.ts

Lines 592 to 600 in 533c4d0

    
           this._operationInProgress = false; 
        
           // Iterate the pending discontinuity events and trigger them 
        
           for (const [contributor, error] of this._pendingDiscontinuityEvents) { 
        
             contributor.discontinuityDetected(error); 
        
           } 
        
           this._pendingDiscontinuityEvents.clear(); 
        
           return attachResult;

While we are still emitting discontinuity events as a part of mutually exclusive/atomic attach op.

I mean, the flag can be simply set to true before starting mtx and set to false after mtx op ends.

Btw, if operationInProgress is only meant for Attach op, we should update doc + variable name for the same.

Yep - agree this needs a re-look.

The main purpose of the flag is to prevent channel events from being considered during operations.

Re the positioning of the set to false - where it happens should be fine practically, but no harm moving it to before the return.

Hey, I didn't want to block you for PR related to this, but I didn't really get what you meant by
#412 (comment)

The place I don't think it works is in the attach cycle - the attach operation ends (and should fail fast back to the user), but we need to continue the lifecycle operation in the background to retry, so in this instance we don't want to tie the lifecycle op to the attach call but to what happens in the background afterwards.

In simple terms, what I understand is operationInProgress=true if there is ongoing op ( external or internal ) + pending operations. When all operations all performed, operationInProgress=false, is that right?

Basically, we should wait for all operations to be performed by _mtx , then only set operationInProgress = false right?

	// This shouldn't be the case except in testing, but if we're already attached, then we should consider
	// ourselves not in the middle of an operation and thus consider channel events.
	if (this._lifecycle.status !== RoomStatus.Attached) {
	this._operationInProgress = true;
	}

	this._operationInProgress = false;

	// Iterate the pending discontinuity events and trigger them
	for (const [contributor, error] of this._pendingDiscontinuityEvents) {
	contributor.discontinuityDetected(error);
	}
	this._pendingDiscontinuityEvents.clear();

	return attachResult;