[robmoffat]

This pull request introduces significant updates to the WebSocket Connection Protocol (WSCP) and enhances the API documentation, particularly by adding Java language bindings. The main changes include a refactor of the WSCP handshake to use role-specific connect messages, the addition of new JSON schema files for WSCP message types, and comprehensive Java examples and interface details in the Channel API reference.

WebSocket Connection Protocol (WSCP) Refactor and Schema Additions:

• Refactored the WSCP handshake to use role-specific connect messages, where the TCP initiator sends the sharedSecret and the acceptor does not echo it. The assigned app identity is now carried in implementationMetadata.appMetadata.
• Added new JSON schema files for WSCP message types: WSCPApplicationConnect, WSCPDesktopAgentConnect, WSCPConnectFailed, WSCPGoodbye, and the shared WSCPConnectionStep schema. These define the structure and requirements for each protocol message, improving clarity and interoperability. [1] [2] [3] [4] [5]

API Documentation Enhancements:

• Added Java language binding documentation to the API reference, including detailed interface definitions, method signatures, and usage examples for the Channel API. This improves accessibility for Java developers and ensures parity with other supported languages. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19]

[Copilot]

🔵 Needs a closer look

A couple of the newly added Java documentation snippets are not self-contained/compilable as written (undefined variable and incomplete enum constructor), which will mislead readers copying examples.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

website/docs/api/ref/Channel.md:207

• This Java enum example will not compile as written because the constants call a Type(String) constructor that isn't defined, so readers copying it will hit an error. Add the constructor (and ideally an accessor for the string value) in the snippet.
  website/docs/api/spec.md:464
• The Java snippet references appIntent but does not declare it, so it isn't self-contained for readers copying the example.

• Files reviewed: 18/19 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

A few Java documentation snippets are inconsistent with the standard’s async semantics and include an invalid fdc3.instrument example, which could mislead implementers.

Review details

Suppressed comments (4)

Previously missed (3) — in code that hasn't changed since the last review.

website/docs/api/ref/PrivateChannel.md:71

• In the Java binding section, PrivateChannel.disconnect() is documented as void, but all other language bindings define disconnect as asynchronous (Promise<void>, Task, etc.) and the Java API elsewhere uses CompletionStage for async operations. Documenting this as synchronous is likely incorrect and prevents callers from awaiting completion/errors/timeouts.

This issue also appears on line 472 of the same file.
website/docs/api/ref/Types.md:520

• The comment in this Java snippet should capitalize the language name and remove the trailing double-space.
  website/docs/api/supported-platforms.md:204
• The Java example broadcasts an fdc3.instrument context without an id, but the fdc3.instrument schema requires at least one identifier in id (e.g. ticker). As written, this example would produce an invalid context payload for many Desktop Agents.

website/docs/api/ref/PrivateChannel.md:474

• This Java method-signature snippet documents disconnect() as returning void, but the API is asynchronous and the Java binding docs consistently use CompletionStage for async operations. Returning CompletionStage<Void> allows callers to await completion and handle failures/timeouts.

```java
void disconnect();

- **Files reviewed:** 18/19 changed files
- **Comments generated:** 0 new
- **Review effort level:** Lite
</details>

[Copilot]

🔵 Needs a closer look

The new WSCP base schema and WSCP spec doc contain correctness gaps (schema permits structurally invalid meta/payload combinations; Flow 2 steps omit the required response message), and one Java code snippet is not syntactically valid as written.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

packages/fdc3-schema/schemas/api/WSCPConnectionStep.schema.json:42

• WSCPConnectionStep currently allows meta to be either DisconnectStepMeta or ConnectionStepMeta for any type, which means a connect message could validate without connectionAttemptUuid and WSCPGoodbye could validate with it (and even with a payload). Since this schema is used as the base WSCP message shape, it should constrain meta/payload based on type to prevent structurally invalid handshakes from passing schema validation.
  website/docs/api/ref/Errors.md:810
• The Java code block contains Not implemented as a bare identifier, which is not valid Java and renders oddly with Java syntax highlighting. Use a comment so the snippet is syntactically valid.
  website/docs/api/specs/webSocketConnectionProtocol.md:242
• In Flow 2, the numbered steps omit the application’s WSCPApplicationConnect response even though the sequence diagram (and earlier text) defines a two-message handshake. As written, readers could interpret that DACP starts immediately after validation without the application sending the response message.

• Files reviewed: 18/19 changed files
• Comments generated: 0 new
• Review effort level: Lite

[robmoffat]

@copilot I'm not changing the things you've put in your review - they're needlessly pedantic. Please submit decent critique or just pass the review

[Copilot]

@copilot I'm not changing the things you've put in your review - they're needlessly pedantic. Please submit decent critique or just pass the review

Understood. I won’t ask for those changes again on this PR.

[kriswest]

I dunno, the first few points seem valid to me

[robmoffat]

I dunno, the first few points seem valid to me

Perhaps it should have brought them up in one of the first three reviews then.

[julianna-ciq]

Reviewing this as the active implementation of FDC3 issue #1792. The Java binding is not ready to standardize yet: the repository linked from the new platform documentation still exposes the legacy API on main, while the API that these pages describe exists only in a closed, unmerged PR. There are also runtime/signature mismatches with that proposed implementation and an unrelated breaking rename in the generated schema package. Please also update the pre-existing statements in supported-platforms.md that say the standard defines only JavaScript/.NET bindings (line 115) and no native binding at all (line 225); both directly contradict the Java binding introduced by this PR.

[julianna-ciq]

[P1] Make the referenced binding available before advertising it here. The linked repository's main branch still has the legacy fdc3api / com.finos... layout and contains none of the fdc3-standard, fdc3-get-agent, GetAgentParams, or org.finos... APIs used below. Those changes exist only in finos-labs/fdc3-java-api#7, which is closed without being merged, and the proposed finos/FDC3-Java repository does not exist yet. As a result, a reader following this page cannot obtain or compile against the binding being standardized. Land/move the implementation (and point this link at it), or defer these usage claims until it is available.

[robmoffat]

Hi @julianna-ciq,

I've just finished updating finos-labs/fdc3-java-api#8 to be consistent with the changes introduced in FDC3 3.0 recently. (see finos-labs/fdc3-java-api#8 (comment))

Hopefully this helps your review

[julianna-ciq]

[P1] Align these enum values with the Java implementation before using them in the examples. The proposed Java source currently defines ON_UNSUBSCRIBE("onUnsubscribe") and ON_DISCONNECT("onDisconnect"), while DefaultPrivateChannel.addEventListener accepts only "unsubscribe" and "disconnect". Consequently every new example that calls FDC3Event.Type.ON_DISCONNECT.getValue() or ON_UNSUBSCRIBE.getValue() is rejected with ChannelError.InvalidArguments at runtime. The enum/proxy and this normative snippet need to agree on the standard event strings.

[robmoffat]

not sure I grasp what this is talking about