Please remove this code, and add this null check to line 749 above.

What should GetMessageGuid() return in the case? Guid.Empty?

Yes. Ensuing data processing will check for null or bad packets and do the appropriate thing. Thanks!

Please add try/catch. This method should not throw.

With the new implementation the method can not throw. But I added explicit try-catch to avoid a regression if anybody change the method.

Both Slice() and IndexOf() methods can throw.

It seems ReadOnlySpan.IndexOf() never throw. Slice() could be but IndexOf() before it protects from this.
In any case I already added try-catch to exclude a regression in future..

Please move string null/empty check to here.

I added the explicit check.

Using Span.Slice() is a good optimization. But this code is actually incorrect as is. All PSRP packets contain a Guid. But a session message guid is always an empty guid (which is why the original code compares to an empty Guid). A non-empty guid means the PSRP packet is a command message. This method is used to route the message to the appropriate processing thread. To fix this, please make the following changes;

private bool IsCommandMessage(ReadOnlySpan<char> data)
{
    try
    {
        // Perform quick scan for data packet GUID.
        var iTag = data.Index(GUIDTAG, StringComparison.OrdinalIgnoreCase);
        if (iTag > -1)
        {
            // An empty GUID value means this is a session message, otherwise it is a command message.
            var psGuidString = data.Slice(iTag + GUIDTag.Length, GUID_STR_LEN);
            if (Guid.TryParse(psGuidString out Guid psGuid))
            {
                return !Guid.Equals(psGuid, Guid.Empty);
            }
        }
    }
    catch
    {
        // The method should never throw.
    }

    // Missing Guid means an invalid packet.  Continue processing as session message.
    return false;
}

Ah, I did know. Thanks!

Do we know that the package integrity already was checked and GUID always present? If yes we could only search the const without parsing:

Yes, good point. We can simply search for an empty PSGuid element. All valid packets will have a PSGuid element, and if invalid we just treat as a session message.

Done.

I think we can make it even simpler. I now agree that the method won't throw so we can remove the try/catch. Also empty Guid equates to session message so we should name the method correctly. I was thinking:

private const string SESSIONGUID = "PSGuid='00000000-0000-0000-0000-000000000000'";
private bool IsSessionMessage(string data)
{
    // Perform quick scan for an empty GUID element, which identifies a session message.
    return data.IndexOf(SESSIONGUID, StringComparison.OrdinalIgnoreCase) > -1;
}

Since this is only used for routing, any invalid packet can be routed to command thread ... it really shouldn't matter.

Done.

Please change to use new IsCommandMessage().

Done.

Please remove this comment.
I mean please just remove this comment:
// Session messages have empty Guid values.
because it seems unnecessary now.

Done.

Minor comment. The null check does not need to be within the try/catch.

Done.

Just curious if we still need to pass as Span type. We can just scan a normal string type, but I am not sure if it really makes a difference.

:-) Yes, but we again would care about null to exclude a regression in future.

I think if you worry that a null value may be passed to this method, the intent is more clear by having a null check here:

private bool IsSessionMessage(string data)
{
    if (data == null) { return false; }
    return data.IndexOf(SESSIONDMESSAGETAG, StringComparison.OrdinalIgnoreCase) > -1;
}

Or, maybe this method should just be directly inlined in HandleOutputDataReceived given it's just one line of code and not used anywhere else.

I prefer having the private method since it makes clear what the purpose is. Null check is not needed.

Inlined the method.

Oops, @PaulHigin sorry I did not see your comment. Perhaps SESSIONDMESSAGETAG well says that the purpose is and inlining is goog? Thoughts?

@PaulHigin Would it be OK to add a comment to the inlined code to make the purpose clear?
I don't mind having it a separate private method, just thought using string data should be sufficient.

I assumed the compiler would inline but I am fine either way. These are minor issues and overall I am happy with the changes.