-
Notifications
You must be signed in to change notification settings - Fork 64
Add ui/close-resource request for UI to initiate termination #215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add ui/close-resource request for UI to initiate termination #215
Conversation
@modelcontextprotocol/ext-apps
@modelcontextprotocol/server-basic-react
@modelcontextprotocol/server-basic-vanillajs
@modelcontextprotocol/server-budget-allocator
@modelcontextprotocol/server-cohort-heatmap
@modelcontextprotocol/server-customer-segmentation
@modelcontextprotocol/server-map
@modelcontextprotocol/server-pdf
@modelcontextprotocol/server-scenario-modeler
@modelcontextprotocol/server-shadertoy
@modelcontextprotocol/server-sheet-music
@modelcontextprotocol/server-system-monitor
@modelcontextprotocol/server-threejs
@modelcontextprotocol/server-transcript
@modelcontextprotocol/server-video-resource
@modelcontextprotocol/server-wiki-explorer
commit: |
df4c168 to
52fff75
Compare
52fff75 to
949f84a
Compare
949f84a to
95bd5ec
Compare
|
@fredericbarthelet thanks for this! several points:
What do you think? |
|
Hey @liady, thanks for your comments. I'll add the spec changes and tests to the PR.
while, given it was at the guest initiative, it could have been much simpler:
For the sake of clarity (and steering away from this conversation analogy 😅), here are the 2 options as sequences. Option A: ui/close-resource (current PR)Simpler. sequenceDiagram
participant Host
participant Guest
note right of Guest: App performs cleanup first
Guest->>Guest: saveState(), cleanup()
Guest--)Host: ui/close-resource (request)
note left of Host: Host unmounts immediately
Host->>Host: iframe.remove()
Option B: ui/request-close → ui/resource-teardownPiggy-back on existing implementation of sequenceDiagram
participant Host
participant Guest
Guest--)Host: ui/request-close (notification)
note left of Host: Host decides whether to close
alt Host approves close
Host->>Guest: ui/resource-teardown (request)
note right of Guest: App performs cleanup
Guest->>Guest: saveState(), cleanup()
Guest-->>Host: {} (success response)
Host->>Host: iframe.remove()
else Host denies close
note left of Host: Host ignores or defers
end
I can go with option B if you deem it more appropriate here. Let me know :) |
Fixes #203
Motivation and Context
Implement bi-directionality on UI termination.
Until now
ui/resource-teardownwas the only way for Host to initiate UI teardown.This methods enable UI to initiate its own teardown
Items that might require discussion in this PR:
open-link,message,request-display-mode, the name of the request should be action-first/verb-first. Even if we implement a fire and forget pattern (see next point), I would not includenotificationsin the name. What do you think ofui/close-resource?ui/close-resourcemessage implements a request/response pattern or a fire and forget type of pattern (where UI fires the event like it would fire a notification without ever expecting an answer). I'm leaning towards fire and forget because the host won't be able to send a response if it proceeds accordingly and does indeed terminates the UIui/close-resourcerequests be fulfilled? this can be achieved only if we go for a request/response pattern, so I'd not account for this scenario just yetTypes of changes
Checklist
Additional context