v2/pkg/engine/datasource/graphql_datasource/graphql_sse_handler.go (1)

161-186: Do not panic on unexpected errors value type; fail gracefully and propagate an internal error

Panic here can take down the goroutine/process and is not appropriate for malformed or non-spec-conforming inputs from upstream. Prefer logging and emitting an internal error on the error channel.

Apply this diff to handle the case safely:

 				case jsonparser.Object:
 					response := []byte(`{"errors":[]}`)
 					response, err = jsonparser.Set(response, val, "errors", "[0]")
 					if err != nil {
 						h.log.Error("failed to set errors", log.Error(err))
 
 						errCh <- []byte(internalError)
 						return
 					}
 					errCh <- response
 					return
-				default:
-					panic(fmt.Sprintf("unexpected value type: %d", valueType))
+				default:
+					// unexpected type for "errors" field; don't crash the handler
+					h.log.Error(fmt.Sprintf("unexpected errors value type: %d", valueType))
+					errCh <- []byte(internalError)
+					return
 				}

v2/pkg/engine/resolve/subscription_filter.go (1)

109-137: Add default case for unknown SegmentType to avoid silent fallthrough

If a new/unknown SegmentType is introduced, valueType remains the zero value (likely NotExist), leading to silent fallthrough and potentially incorrect comparisons. Return an explicit error to surface invalid templates early.

Apply this diff:

 				switch f.Values[i].Segments[0].SegmentType {
 				case VariableSegmentType:
 					value := ctx.Variables.Get(f.Values[i].Segments[0].VariableSourcePath...)
 					if value == nil {
 						return true, nil
 					}
 					switch value.Type() {
 					case astjson.TypeString:
 						valueType = jsonparser.String
 					case astjson.TypeNumber:
 						valueType = jsonparser.Number
 					case astjson.TypeTrue, astjson.TypeFalse:
 						valueType = jsonparser.Boolean
 					case astjson.TypeNull:
 						valueType = jsonparser.Null
 					case astjson.TypeObject:
 						valueType = jsonparser.Object
 					case astjson.TypeArray:
 						valueType = jsonparser.Array
 					default:
 						return true, nil
 					}
 				case StaticSegmentType:
 					_, valueType, _, err = jsonparser.Get(f.Values[i].Segments[0].Data)
 					if err != nil {
 						return true, nil
 					}
+				default:
+					// Defensive: segment type not recognized – surface this as an invalid template
+					return true, InvalidSubscriptionFilterTemplate
 				}
 
 				if valueType != jsonparser.NotExist && expectedDataType != valueType {
 					return true, nil
 				}

Also applies to: 185-186

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

v2/pkg/engine/datasource/graphql_datasource/graphql_sse_handler.go (1)

161-186: Option: handle json null explicitly to avoid treating it as an unexpected type

If upstream sends "errors": null, treating it as "not found" (skip) instead of unexpected can be friendlier. Consider a dedicated case.

Example:

                 switch valueType {
                 case jsonparser.Array:
                     ...
                 case jsonparser.Object:
                     ...
+                case jsonparser.Null:
+                    // treat null like not present; ignore and continue
+                    continue
                 default:
                     ...

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

v2/pkg/engine/datasource/graphql_datasource/graphql_sse_handler_test.go (4)

352-407: Subtests could run in parallel; capture the range variable to avoid the classic gotcha.

Optional, but this shortens runtime and isolates cases. If you add t.Parallel(), also capture tc to avoid closure over the loop variable.

Apply:

-for _, tc := range testCases {
-  t.Run(tc.name, func(t *testing.T) {
+for _, tc := range testCases {
+  tc := tc
+  t.Run(tc.name, func(t *testing.T) {
+    t.Parallel()

---

365-369: Confirm that sending SSE frames without the "data:" prefix is intentional and supported.

SSE-compliant frames use "data:" (and optionally "event:"). This test intentionally omits "data:", relying on non-standard behavior. If the handler depends on this to exercise the value-type switch, keep it; otherwise, prefer spec-conformant frames to reduce test fragility.

If you decide to make it spec-compliant, adjust as:

-// Send the malformed error message WITHOUT the "data:" prefix
-// This triggers the error parsing logic in the default case
-_, _ = fmt.Fprintf(w, "%s\n\n", tc.errorMessage)
+// Send the error payload as a proper SSE frame
+_, _ = fmt.Fprintf(w, "data: %s\n\n", tc.errorMessage)

---

372-372: Use t.Cleanup for server shutdown.

Slightly more idiomatic in subtests and guarantees cleanup even if the subtest ends early.

-      defer server.Close()
+      t.Cleanup(server.Close)

---

396-399: Use JSON-aware equality for robustness.

Comparing JSON as strings is brittle (whitespace, key order). Testify provides JSONEq.

-      assert.Equal(t, 1, len(updater.updates))
-      assert.Equal(t, tc.expectedErr, updater.updates[0])
+      assert.Equal(t, 1, len(updater.updates))
+      assert.JSONEq(t, tc.expectedErr, updater.updates[0])

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.