Skip to content

Commit 8fde19b

Browse files
Copilotbaywettimotheeguerin
authored
Validate SSEStream union parameter has @events decorator (#8951)
SSEStream accepted unions without `@events` decorator, generating incorrect schemas. This validation ensures proper event stream definitions at compile time. ## Changes - **Added validation** in `packages/sse/src/validate.ts`: - Traverse all models during validation phase - Check streams with `text/event-stream` content-type - Verify streamOf type is a union with `@events` decorator - Report `sse-stream-union-not-events` diagnostic on failure - **Added diagnostic** `sse-stream-union-not-events` in `packages/sse/src/lib.ts` - **Added tests** covering invalid/valid SSEStream usage and HttpStream with SSE content-type ## Example This now fails validation: ```typespec union BasicUnion { userconnect: UserConnect, } op subscribe(): SSEStream<BasicUnion>; // Error: union must have @events ``` This compiles: ```typespec @events union BasicUnion { userconnect: UserConnect, } op subscribe(): SSEStream<BasicUnion>; // OK ``` Fixes #8888 > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `telemetry.astro.build` > - Triggering command: `node /home/REDACTED/work/typespec/typespec/website/node_modules/.bin/../astro/astro.js build` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/microsoft/typespec/settings/copilot/coding_agent) (admins only) > > </details> <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>SSEStream should NOT accept a union model without the events decorator</issue_title> > <issue_description>This definition > > ```ts > import "@typespec/streams"; > import "@typespec/sse"; > import "@typespec/events"; > > using SSE; > > > model UserConnect { > name: string; > } > > union BasicUnion { > userconnect: UserConnect, > } > op subscribe(): SSEStream<BasicUnion>; > ``` > > Builds but generates the wrong schema (instead of item schema with all the right properties) > > ```ts > import "@typespec/streams"; > import "@typespec/sse"; > import "@typespec/events"; > > using SSE; > > > model UserConnect { > name: string; > } > > @TypeSpec.Events.events > union BasicUnion { > userconnect: UserConnect, > } > op subscribe(): SSEStream<BasicUnion>; > ``` > > Generates the correct itemSchema. > > Ultimately this is not a responsibility of the OpenAPI3 emitter to check for this, but rather the SSE library should fail building when being passed a union model as a generic type parameter value when that union model DOES NOT have the required decorator. > This will be handled in a separate issue/pull request > > _Originally posted by @baywet in #8888 (comment) > </issue_description> > > <agent_instructions>before committing any file, run `npx prettier --write filePath` to ensure they are formatted correctly. > once you're done with the changes, run `npx @chronus/chronus add` and select bugfix to add an entry in the changelog</agent_instructions> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > <comment_new><author>@timotheeguerin</author><body> > Also note that as `SSEStream` is just defined as `HttpStream<Type, "text/event-stream">;` > > that also might need to validate when `text/event-stream` is used</body></comment_new> > </comments> > </details> - Fixes #8950 <!-- START COPILOT CODING AGENT TIPS --> --- 💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs. --------- Signed-off-by: Vincent Biret <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: baywet <[email protected]> Co-authored-by: Vincent Biret <[email protected]> Co-authored-by: Timothee Guerin <[email protected]>
1 parent 1dde0d1 commit 8fde19b

File tree

4 files changed

+152
-3
lines changed

4 files changed

+152
-3
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
changeKind: fix
3+
packages:
4+
- "@typespec/sse"
5+
---
6+
7+
Validate SSE streams are defined as events unions

packages/sse/src/lib.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,14 @@ export const $lib = createTypeSpecLibrary({
77
severity: "error",
88
messages: {
99
default:
10-
"A field marked as '@terminalEvent' must be a member of a type decorated with '@TpeSpec.Events.events'.",
10+
"A field marked as '@terminalEvent' must be a member of a type decorated with '@TypeSpec.Events.events'.",
11+
},
12+
},
13+
"sse-stream-union-not-events": {
14+
severity: "error",
15+
messages: {
16+
default:
17+
"SSEStream type parameter must be a union decorated with '@TypeSpec.Events.events'.",
1118
},
1219
},
1320
},

packages/sse/src/validate.ts

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1-
import type { Program, UnionVariant } from "@typespec/compiler";
1+
import type { Model, Program, UnionVariant } from "@typespec/compiler";
2+
import { navigateProgram } from "@typespec/compiler";
23
import { isEvents } from "@typespec/events";
4+
import { getContentTypes } from "@typespec/http";
5+
import { getStreamOf } from "@typespec/streams";
36
import { reportDiagnostic, SSEStateKeys } from "./lib.js";
47

58
export function $onValidate(program: Program) {
69
checkForIncorrectlyAssignedTerminalEvents(program);
10+
checkForSSEStreamWithoutEventsDecorator(program);
711
}
812

913
function checkForIncorrectlyAssignedTerminalEvents(program: Program) {
@@ -24,3 +28,49 @@ export function validateTerminalEvent(program: Program, target: UnionVariant) {
2428
});
2529
}
2630
}
31+
32+
function checkForSSEStreamWithoutEventsDecorator(program: Program) {
33+
navigateProgram(program, {
34+
model: (model) => {
35+
validateSSEStream(program, model);
36+
},
37+
});
38+
}
39+
40+
function validateSSEStream(program: Program, model: Model) {
41+
// Check if this model is a stream
42+
const streamOf = getStreamOf(program, model);
43+
if (!streamOf) {
44+
return;
45+
}
46+
47+
// Check if the stream has the text/event-stream content type
48+
const contentTypeProperty = model.properties.get("contentType");
49+
if (!contentTypeProperty) {
50+
return;
51+
}
52+
53+
const [contentTypes] = getContentTypes(contentTypeProperty);
54+
const isSSEStream = contentTypes.includes("text/event-stream");
55+
56+
if (!isSSEStream) {
57+
return;
58+
}
59+
60+
// The stream is an SSEStream, validate that streamOf is a union with @events
61+
if (streamOf.kind !== "Union") {
62+
// If streamOf is not a union, it's invalid for SSE streams
63+
reportDiagnostic(program, {
64+
code: "sse-stream-union-not-events",
65+
target: model,
66+
});
67+
return;
68+
}
69+
70+
if (!isEvents(program, streamOf)) {
71+
reportDiagnostic(program, {
72+
code: "sse-stream-union-not-events",
73+
target: model,
74+
});
75+
}
76+
}

packages/sse/test/models.test.ts

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { t } from "@typespec/compiler/testing";
1+
import { expectDiagnostics, t } from "@typespec/compiler/testing";
22
import { getContentTypes } from "@typespec/http";
33
import { getStreamOf } from "@typespec/streams";
44
import { describe, expect, it } from "vitest";
@@ -23,4 +23,89 @@ describe("SSEStream", () => {
2323
name: "string",
2424
});
2525
});
26+
27+
it("should fail when union is not decorated with @events", async () => {
28+
const diagnostics = await Tester.diagnose(`
29+
model UserConnect {
30+
name: string;
31+
}
32+
33+
union BasicUnion {
34+
userconnect: UserConnect,
35+
}
36+
37+
op subscribe(): SSEStream<BasicUnion>;
38+
`);
39+
40+
expectDiagnostics(diagnostics, {
41+
code: "@typespec/sse/sse-stream-union-not-events",
42+
severity: "error",
43+
});
44+
});
45+
46+
it("should pass when union is decorated with @events", async () => {
47+
const diagnostics = await Tester.diagnose(`
48+
model UserConnect {
49+
name: string;
50+
}
51+
52+
@events
53+
union BasicUnion {
54+
userconnect: UserConnect,
55+
}
56+
57+
op subscribe(): SSEStream<BasicUnion>;
58+
`);
59+
60+
expectDiagnostics(diagnostics, []);
61+
});
62+
63+
it("should fail when HttpStream with text/event-stream is used without @events", async () => {
64+
const diagnostics = await Tester.diagnose(`
65+
model UserConnect {
66+
name: string;
67+
}
68+
69+
union BasicUnion {
70+
userconnect: UserConnect,
71+
}
72+
73+
model MyStream is Http.Streams.HttpStream<BasicUnion, "text/event-stream">;
74+
`);
75+
76+
expectDiagnostics(diagnostics, {
77+
code: "@typespec/sse/sse-stream-union-not-events",
78+
severity: "error",
79+
});
80+
});
81+
82+
it("should fail when HttpStream with text/event-stream is a model", async () => {
83+
const diagnostics = await Tester.diagnose(`
84+
model UserConnect {
85+
name: string;
86+
}
87+
88+
model MyStream is Http.Streams.HttpStream<UserConnect, "text/event-stream">;
89+
`);
90+
91+
expectDiagnostics(diagnostics, {
92+
code: "@typespec/sse/sse-stream-union-not-events",
93+
severity: "error",
94+
});
95+
});
96+
97+
it("should fail when SSEStream is used with a model", async () => {
98+
const diagnostics = await Tester.diagnose(`
99+
model UserConnect {
100+
name: string;
101+
}
102+
103+
model MyStream is SSEStream<UserConnect>;
104+
`);
105+
106+
expectDiagnostics(diagnostics, {
107+
code: "invalid-argument",
108+
severity: "error",
109+
});
110+
});
26111
});

0 commit comments

Comments
 (0)