RaiseIntentResultResponse

[robmoffat]

Minor Issue

The da-proxy is currently sending a RaiseIntentResultResponse to the Desktop Agent (server) when an intent listener returns a result.

This doesn't seem quite correct, since in your schema generally messages ending with Response are from the DA back to the app...

So should there be a RaiseIntentResultRequest message? Or should I just use the same message in both directions? (App->Da and Da->Raising App)

Edit 1

Ok, so here's a mermaid diagram, I think there are actually a couple of missing messages which I want to clear up:

sequenceDiagram
    AppA->>DesktopAgent: raiseIntentRequest
    DesktopAgent ->> AppB: open the app
    AppB ->> DesktopAgent: addIntentListenerRequest
    DesktopAgent ->> AppB: addIntentListenerResponse
    DesktopAgent ->> AppB: intentEvent
    DesktopAgent ->> AppA: raiseIntentResponse
    note right of AppA: contains IntentResolution - see Note 1
    AppB ->> DesktopAgent:intentResultRequest (?)
    note left of AppB: doesn't exist - see Note 2
    DesktopAgent ->> AppA: raiseIntentResultResponse

Note 1:

RaiseIntentResponsePayload looks like this:

export interface RaiseIntentResponsePayload {
    error?:            FindInstancesErrors;
    intentResolution?: IntentResolution;
    appIntent?:        AppIntent;
}

I'm not sure what AppIntent is doing in there - what's the purpose of this?

Note 2

As mentioned, this should have a payload similar to RaiseIntentResponsePayload but for the App to send.

[kriswest]

You are correct that we're missing a message for the result from the intent handler. Happy to add a intentResultRequest - every other request has a response so perhaps we should add one so the proxy can log that it got acknowledgement that the result was delivered? I'll try and add these today.

Regarding Note 1

This is the result of QuickType merging the 3 possible responses - if you are not seeing docs in that type explaining then I need to move the comments from the payloads to the fields - confirmed I'll make that move.

Basically, there are 3 possible responses: https://github.com/finos/FDC3/blob/69a4c08316077f4ffabf548b6e08ff8416143ecb/schemas/api/raiseIntentResponse.schema.json#L17-L91

1. Successfully resolved (targetted or there was only one option): intentResolution
2. Requires resolution - show the intent resolver UI appIntent -. AppIntent encodes all the data needed on what options are available, including the metadata for each app (display name, logo etc.)
3. An error: error

So in any response only one of those should be set. QUicktype isn't quite capable of creating a union type that would ensure only one was set - but that is exactly how it is in the JSON Schemas, which are your source of truth.

Regarding Note 2

As commented above I'll try and add a request/response for that today. Although it will be near identical to raiseIntentResultResponse, it makes sense to keep things consistent!

[kriswest]

@robmoffat should be fixed by 70ea90f5f92e9025717f4b771afd2a7a3c10569f - I've also made sure you get docs on the types for those properties to make it clear which should be set when.

[robmoffat]

Ok, I realise I am being a PITA but I think we need an IntentResultEvent message too. That way, we can use IntentResultResponse to go back to the proxy that is sending the intent result, like so:

sequenceDiagram
    AppA ->> DesktopAgent: raiseIntentRequest
    DesktopAgent ->> AppB: intentEvent
    DesktopAgent ->> AppA: raiseIntentResponse
    AppB ->> DesktopAgent:intentResultRequest
    DesktopAgent ->> AppA: intentResultEvent
    DesktopAgent ->> AppB: intentResultResponse

[kriswest]

@robmoffat IntentResultResponse already exists and is used for that purpose. You're looking for raiseIntentResultResponse for the delivery (same pattern that we used in bridging - and naming is consistent this way). I would also just go ahead and return the intentResultResponse to the resolving app immediately - it doesn't need confirmation that it was passed onto the raising app as it might never bother picking it up or may have been closed). Hence, the current sequence diagram should be:

sequenceDiagram
    AppA ->> DesktopAgent: raiseIntentRequest
    DesktopAgent ->> AppB: intentEvent
    DesktopAgent ->> AppA: raiseIntentResponse
    AppB ->> DesktopAgent:intentResultRequest
    DesktopAgent ->> AppB: intentResultResponse
    DesktopAgent ->> AppA: raiseIntentResultResponse

Closing as I think this works already, reopen if not.

[kriswest]

P.S. no worries on being a PITA about it - its a protocol and we've got to get it right!

[robmoffat]

I'm getting some errors on the new schemas:

strict mode: missing type "object" for keyword "required" at "https://fdc3.finos.org/schemas/next/api/raiseIntentResultResponse.schema.json#" (strictTypes) strict mode: missing type "object" for keyword "additionalProperties" at "https://fdc3.finos.org/schemas/next/api/raiseIntentResultResponse.schema.json#" (strictTypes) strict mode: missing type "object" for keyword "properties" at "https://fdc3.finos.org/schemas/next/api/raiseIntentResultResponse.schema.json#" (strictTypes)

Does this mean anything to you?

[kriswest]

WHere are you getting those errors/what from? I don't recognise them

[kriswest]

Looks like thats an error from ajv in strict mode - I may know whats up....There was a missing type: "object" statement in one of the payload options. Should be fixed in 809647324ec2f51df4a0a6fdbc2132ce5f97c7be

Send me any more of those it reports.