ui/src/features/dags/components/dag-details/DAGStepTableRow.tsx (1)

13-60: Consider centralizing getExecutorCommand to avoid drift.
The helper is duplicated in NodeStatusTableRow.tsx; a shared util would keep behavior aligned across views.

internal/runtime/builtin/redis/command.go (3)

35-43: Consider handling KeepTTL option for SET command.

The Config struct has a KeepTTL field, but the SET command implementation doesn't use it. When KeepTTL is true, the existing TTL should be preserved.

♻️ Suggested enhancement

 	case "SET":
 		ttl := time.Duration(h.cfg.TTL) * time.Second
+		if h.cfg.KeepTTL {
+			return h.client.SetArgs(ctx, h.cfg.Key, h.cfg.Value, redis.SetArgs{
+				KeepTTL: true,
+			}).Result()
+		}
 		if h.cfg.NX {
 			return h.client.SetNX(ctx, h.cfg.Key, h.cfg.Value, ttl).Result()
 		}

---

563-588: Consider adding a safeguard for SCAN on large datasets.

The executeScan method collects all matching keys into memory without any limit. On databases with millions of keys, this could cause excessive memory usage and long execution times.

♻️ Suggested safeguard

 func (h *CommandHandler) executeScan(ctx context.Context) ([]string, error) {
 	var allKeys []string
 	var cursor uint64
 	count := int64(h.cfg.Count)
 	if count == 0 {
 		count = 100
 	}
 	match := h.cfg.Match
 	if match == "" {
 		match = "*"
 	}
+	
+	const maxKeys = 100000 // Configurable limit
 
 	for {
+		if len(allKeys) >= maxKeys {
+			return allKeys, fmt.Errorf("scan result limit reached (%d keys)", maxKeys)
+		}
 		keys, nextCursor, err := h.client.Scan(ctx, cursor, match, count).Result()
 		if err != nil {
 			return nil, err
 		}
 		allKeys = append(allKeys, keys...)
 		cursor = nextCursor
 		if cursor == 0 {
 			break
 		}
 	}
 	return allKeys, nil
 }

---

602-620: Consider handling unsigned integer types in toInt64.

The toInt64 function doesn't handle uint, uint32, or uint64 types. Values unmarshaled from JSON or received from other sources could be unsigned, causing conversion errors.

♻️ Suggested enhancement

 func toInt64(v any) (int64, error) {
 	switch val := v.(type) {
 	case int:
 		return int64(val), nil
 	case int32:
 		return int64(val), nil
 	case int64:
 		return val, nil
+	case uint:
+		return int64(val), nil
+	case uint32:
+		return int64(val), nil
+	case uint64:
+		return int64(val), nil
 	case float64:
 		return int64(val), nil
 	case float32:
 		return int64(val), nil
 	case string:
 		return strconv.ParseInt(val, 10, 64)
 	default:
 		return 0, fmt.Errorf("cannot convert %T to int64", v)
 	}
 }

internal/runtime/builtin/redis/redis_test.go (2)

274-290: Consider using t.Cleanup() for more reliable test cleanup.

The cleanup step ignores errors, which is fine, but using t.Cleanup() would ensure cleanup runs even if the test fails or panics earlier.

♻️ Suggested improvement

+	// Register cleanup first
+	t.Cleanup(func() {
+		delStep := core.Step{
+			Name: "test-del",
+			ExecutorConfig: core.ExecutorConfig{
+				Type: "redis",
+				Config: map[string]any{
+					"host":    host,
+					"command": "DEL",
+					"key":     "test:integration:key",
+				},
+			},
+		}
+		delExec, _ := newRedisExecutor(ctx, delStep)
+		_ = delExec.Run(ctx)
+	})
+
 	// SET a value
 	setStep := core.Step{

---

420-427: Consider adding test cases for jsonl and csv output formats.

The test only covers json and raw formats, but the implementation supports jsonl and csv as well. Adding these would improve coverage of the output formatting logic.

internal/runtime/builtin/redis/config.go (1)

166-227: Validation is comprehensive but could validate TLS certificate pairs.

The validation covers all critical fields. However, if TLSCert is provided, TLSKey should also be required (and vice versa) to prevent misconfiguration.

♻️ Suggested enhancement

+	// Validate TLS certificate pair
+	if (c.TLSCert != "" && c.TLSKey == "") || (c.TLSCert == "" && c.TLSKey != "") {
+		return fmt.Errorf("both tlsCert and tlsKey must be provided together")
+	}
+
 	// Must have a command, script, or pipeline
 	if c.Command == "" && c.Script == "" && c.ScriptFile == "" && len(c.Pipeline) == 0 {

internal/runtime/builtin/redis/result.go (1)

273-304: Consider removing unused specialized write methods or consolidating with writeCSV.

WriteZSet, WriteHash, and WriteList duplicate logic already present in writeCSV. If these methods aren't used externally, they could be removed to reduce maintenance burden.

If these are intended for external use, consider having writeCSV delegate to these methods to avoid duplication.

internal/runtime/builtin/redis/client.go (1)

145-149: Explicitly set a minimum TLS version.
Leaving MinVersion unset makes the negotiated protocol depend on defaults. Consider pinning TLS 1.2+ (or TLS 1.3 if your servers support it) to enforce a baseline.

🔒 Suggested hardening

-	tlsConfig := &tls.Config{
-		InsecureSkipVerify: cfg.TLSSkipVerify, //nolint:gosec // User-controlled option
-	}
+	tlsConfig := &tls.Config{
+		MinVersion:         tls.VersionTLS12,
+		InsecureSkipVerify: cfg.TLSSkipVerify, //nolint:gosec // User-controlled option
+	}