fix: no chat panel when the host has no chat backend - #50
Merged
Merged
Conversation
`workflowFeaturesModule` binds the chat panel as an `IDiagramStartup`, so GLSP
instantiates it eagerly rather than on first use. With no chat backend behind
the diagram, that panel opened by itself, sent its first message to a host with
no handler registered for it, logged
Received notification with unknown method: dialogram/chat/toHost
and gave up five seconds later with
[chat-panel] TIMEOUT: no chat.connectionStatus received within 5s
leaving a chat button that could never answer.
`DiagramProfile.chat` is documented as optional — "when absent the chat backend
is not activated" — and this module's own header says it is the stock product's
feature set, which a custom-view consumer replaces with its own. So a consumer
that composes these features without configuring chat is a normal thing to be,
and that case has to read as "no panel" rather than as a panel that fails.
The flag is derived by the platform from `profile.chat` rather than supplied by
a product. The client and host have to agree about this, and a field a product
sets by hand is one it can set wrongly; `chat` being absent is exactly what "no
chat backend" means, so it is the honest source. Absent is treated as present,
so a host predating the flag behaves as it did.
Two existing tests needed deciding rather than fixing.
The twin test compares the toolkit's product-facing `SidecarClientBehavior`
against the platform's declaration, so a product-supplied field cannot go
missing on the way to the webview. A derived field is the opposite case: it is
filled in during forwarding, and declaring it in a product-facing type would
invite someone to set it by hand, which is the one way the two halves could
disagree. It is excluded by name, with a new assertion that it really is absent
from the product type — an exclusion is only honest if the field is not there.
The stock-HTML snapshot changed because the identifier now carries the flag.
Updated deliberately; the diff is that one field and nothing else.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
workflowFeaturesModulebinds the chat panel as anIDiagramStartup, so GLSP instantiates it eagerly rather than on first use. With no chat backend behind the diagram, the panel opened by itself, sent its first message to a host with no handler registered for it, and logged:leaving a chat button that could never answer.
This is a supported combination, not a misuse.
DiagramProfile.chatis optional and documented as "when absent the chat backend is not activated", and this module's own header describes it as the stock product's feature set that a custom-view consumer replaces. So a consumer composing these features without configuring chat is a normal thing to be, and that case has to read as no panel rather than as a panel that fails.The change
DiagramClientBehavior.chatBackend, derived by the platform fromprofile.chatand forwarded with the rest of the behaviour. The feature module skips the three chat bindings when it isfalse; every other binding in the module is untouched.Derived rather than product-supplied on purpose: the client and the host have to agree, and a field a product sets by hand is one it can set wrongly.
chatbeing absent is exactly what "no chat backend" means, so it is the honest source. Absent is treated as present, so a host predating the flag behaves as it did.Two existing tests needed deciding, not fixing
The twin test compares the toolkit's product-facing
SidecarClientBehavioragainst the platform's declaration, so a product-supplied field cannot go missing on the way to the webview. A derived field is the opposite case: it is filled in during forwarding, and declaring it in a product-facing type would invite someone to set it by hand — the one way the two halves could then disagree. It is excluded by name, with a new assertion that it really is absent from the product type, since an exclusion is only honest if the field is not there.The stock-HTML snapshot changed because the identifier now carries the flag. Updated deliberately; the diff is that one field and nothing else.
Verified
npm run build,npm run typecheck(5/5),npm run check:neutrality(5/5),npm test— all green by exit code.packages/diagram-client/test/chat-panel-requires-a-backend.test.ts, covering backend present, flag absent, and backend absent.