Fix schedule update removing cron/spec when not provided (issue #927)

- Describe current schedule before building update and merge CLI with existing spec
- Preserve CronExpressions/Calendars when --cron and --calendar not passed
- Preserve Intervals when --interval not passed
- Preserve Jitter, TimeZoneName, StartAt, EndAt when corresponding flags not set
  (fully incremental update using FlagSet.Changed)
- Add toScheduleSpecForUpdate(spec, existing); create path still uses toScheduleSpec
- Add tests: TestSchedule_Update_PreservesCronWhenNotProvided,
  TestSchedule_Update_PreservesJitterWhenNotProvided

Author: billrich2001

internal/temporalcli/commands.schedule.go

@@ -221,31 +221,82 @@ func toIntervalSpec(str string) (client.ScheduleIntervalSpec, error) {
 }
 
 func (c *ScheduleConfigurationOptions) toScheduleSpec(spec *client.ScheduleSpec) error {
-	spec.CronExpressions = c.Cron
-	// Skip not supported
-	spec.Jitter = c.Jitter.Duration()
-	spec.TimeZoneName = c.TimeZone
-	spec.StartAt = c.StartTime.Time()
-	spec.EndAt = c.EndTime.Time()
+	return c.toScheduleSpecForUpdate(spec, nil)
+}
 
-	var err error
-	for _, calPbStr := range c.Calendar {
-		var calPb schedpb.CalendarSpec
-		if err = protojson.Unmarshal([]byte(calPbStr), &calPb); err != nil {
-			return fmt.Errorf("failed to parse json calendar spec: %w", err)
+// toScheduleSpecForUpdate applies CLI options to spec, preserving existing spec
+// fields when the user did not pass the corresponding flags (fully incremental update).
+// If existing is nil, behaves like toScheduleSpec (no preservation).
+func (c *ScheduleConfigurationOptions) toScheduleSpecForUpdate(spec *client.ScheduleSpec, existing *client.ScheduleSpec) error {
+	// Jitter, TimeZoneName, StartAt, EndAt: preserve from existing when flag not set
+	if existing != nil && c.FlagSet != nil {
+		if !c.FlagSet.Changed("jitter") {
+			spec.Jitter = existing.Jitter
+		} else {
+			spec.Jitter = c.Jitter.Duration()
 		}
-		cron, err := toCronString(&calPb)
-		if err != nil {
-			return err
+		if !c.FlagSet.Changed("time-zone") {
+			spec.TimeZoneName = existing.TimeZoneName
+		} else {
+			spec.TimeZoneName = c.TimeZone
+		}
+		if !c.FlagSet.Changed("start-time") {
+			spec.StartAt = existing.StartAt
+		} else {
+			spec.StartAt = c.StartTime.Time()
+		}
+		if !c.FlagSet.Changed("end-time") {
+			spec.EndAt = existing.EndAt
+		} else {
+			spec.EndAt = c.EndTime.Time()
+		}
+	} else {
+		spec.Jitter = c.Jitter.Duration()
+		spec.TimeZoneName = c.TimeZone
+		spec.StartAt = c.StartTime.Time()
+		spec.EndAt = c.EndTime.Time()
+	}
+
+	// Cron/calendar: preserve from existing when not provided by CLI
+	if len(c.Cron) == 0 && len(c.Calendar) == 0 {
+		if existing != nil {
+			spec.CronExpressions = append([]string(nil), existing.CronExpressions...)
+			spec.Calendars = append([]client.ScheduleCalendarSpec(nil), existing.Calendars...)
+		} else {
+			spec.CronExpressions = nil
+			spec.Calendars = nil
+		}
+	} else {
+		spec.CronExpressions = append([]string(nil), c.Cron...)
+		var err error
+		for _, calPbStr := range c.Calendar {
+			var calPb schedpb.CalendarSpec
+			if err = protojson.Unmarshal([]byte(calPbStr), &calPb); err != nil {
+				return fmt.Errorf("failed to parse json calendar spec: %w", err)
+			}
+			cron, err := toCronString(&calPb)
+			if err != nil {
+				return err
+			}
+			spec.CronExpressions = append(spec.CronExpressions, cron)
 		}
-		spec.CronExpressions = append(spec.CronExpressions, cron)
 	}
-	for _, intStr := range c.Interval {
-		int, err := toIntervalSpec(intStr)
-		if err != nil {
-			return err
+
+	// Intervals: preserve from existing when not provided by CLI
+	if len(c.Interval) == 0 {
+		if existing != nil {
+			spec.Intervals = append([]client.ScheduleIntervalSpec(nil), existing.Intervals...)
+		} else {
+			spec.Intervals = nil
+		}
+	} else {
+		for _, intStr := range c.Interval {
+			intv, err := toIntervalSpec(intStr)
+			if err != nil {
+				return err
+			}
+			spec.Intervals = append(spec.Intervals, intv)
 		}
-		spec.Intervals = append(spec.Intervals, int)
 	}
 
 	return nil
@@ -540,13 +591,18 @@ func (c *TemporalScheduleUpdateCommand) run(cctx *CommandContext, args []string)
 		newSchedule.State.RemainingActions = c.RemainingActions
 	}
 
-	if err = c.toScheduleSpec(newSchedule.Spec); err != nil {
+	sch := cl.ScheduleClient().GetHandle(cctx, c.ScheduleId)
+	description, err := sch.Describe(cctx)
+	if err != nil {
+		return err
+	}
+
+	if err = c.toScheduleSpecForUpdate(newSchedule.Spec, description.Schedule.Spec); err != nil {
 		return err
 	} else if newSchedule.Action, err = toScheduleAction(&c.SharedWorkflowStartOptions, &c.PayloadInputOptions); err != nil {
 		return err
 	}
 
-	sch := cl.ScheduleClient().GetHandle(cctx, c.ScheduleId)
 	return sch.Update(cctx, client.ScheduleUpdateOptions{
 		DoUpdate: func(u client.ScheduleUpdateInput) (*client.ScheduleUpdate, error) {
 			// replace whole schedule

internal/temporalcli/commands.schedule_test.go

@@ -9,6 +9,7 @@ import (
 	"io"
 	"math/rand"
 	"regexp"
+	"strings"
 	"time"
 
 	"github.com/stretchr/testify/assert"
@@ -510,6 +511,105 @@ func (s *SharedServerSuite) TestSchedule_Update() {
 	}, 10*time.Second, 100*time.Millisecond)
 }
 
+func (s *SharedServerSuite) TestSchedule_Update_PreservesCronWhenNotProvided() {
+	schedId, schedWfId, res := s.createSchedule("--cron", "0 9 * * *")
+	s.NoError(res.Err)
+
+	// Update with only non-spec flags (no --cron). Existing cron should be preserved.
+	res = s.Execute(
+		"schedule", "update",
+		"--address", s.Address(),
+		"-s", schedId,
+		"--task-queue", "SomeOtherTq",
+		"--type", "SomeOtherWf",
+		"--workflow-id", schedWfId,
+		"--overlap-policy", "AllowAll",
+	)
+	s.NoError(res.Err)
+
+	s.Eventually(func() bool {
+		res = s.Execute(
+			"schedule", "describe",
+			"--address", s.Address(),
+			"-s", schedId,
+			"-o", "json",
+		)
+		s.NoError(res.Err)
+		var j struct {
+			Schedule struct {
+				Spec struct {
+					CronExpressions []string `json:"cronExpressions"`
+				} `json:"spec"`
+				Action struct {
+					StartWorkflow struct {
+						WorkflowType struct {
+							Name string `json:"name"`
+						} `json:"workflowType"`
+						TaskQueue struct {
+							Name string `json:"name"`
+						} `json:"taskQueue"`
+					} `json:"startWorkflow"`
+				} `json:"action"`
+			} `json:"schedule"`
+			Info struct {
+				FutureActionTimes []string `json:"futureActionTimes"`
+			} `json:"info"`
+		}
+		s.NoError(json.Unmarshal(res.Stdout.Bytes(), &j))
+		// Cron was preserved: either cronExpressions contains our cron, or futureActionTimes
+		// are at 09:00 UTC (server may return structuredCalendar instead of cronExpressions).
+		cronPreserved := (len(j.Schedule.Spec.CronExpressions) == 1 && j.Schedule.Spec.CronExpressions[0] == "0 9 * * *") ||
+			(len(j.Info.FutureActionTimes) > 0 && (strings.Contains(j.Info.FutureActionTimes[0], "T09:00") || strings.Contains(j.Info.FutureActionTimes[0], " 09:00")))
+		return cronPreserved &&
+			j.Schedule.Action.StartWorkflow.WorkflowType.Name == "SomeOtherWf" &&
+			j.Schedule.Action.StartWorkflow.TaskQueue.Name == "SomeOtherTq"
+	}, 10*time.Second, 100*time.Millisecond)
+}
+
+func (s *SharedServerSuite) TestSchedule_Update_PreservesJitterWhenNotProvided() {
+	schedId, schedWfId, res := s.createSchedule("--interval", "10d", "--jitter", "1m")
+	s.NoError(res.Err)
+
+	// Update with only non-spec flags (no --jitter). Existing jitter should be preserved.
+	res = s.Execute(
+		"schedule", "update",
+		"--address", s.Address(),
+		"-s", schedId,
+		"--task-queue", "SomeOtherTq",
+		"--type", "SomeOtherWf",
+		"--workflow-id", schedWfId,
+	)
+	s.NoError(res.Err)
+
+	s.Eventually(func() bool {
+		res = s.Execute(
+			"schedule", "describe",
+			"--address", s.Address(),
+			"-s", schedId,
+			"-o", "json",
+		)
+		s.NoError(res.Err)
+		var j struct {
+			Schedule struct {
+				Spec struct {
+					Jitter string `json:"jitter"`
+				} `json:"spec"`
+				Action struct {
+					StartWorkflow struct {
+						WorkflowType struct {
+							Name string `json:"name"`
+						} `json:"workflowType"`
+					} `json:"startWorkflow"`
+				} `json:"action"`
+			} `json:"schedule"`
+		}
+		s.NoError(json.Unmarshal(res.Stdout.Bytes(), &j))
+		// Jitter 1m is stored as 60s by the server
+		return (j.Schedule.Spec.Jitter == "60s" || j.Schedule.Spec.Jitter == "1m") &&
+			j.Schedule.Action.StartWorkflow.WorkflowType.Name == "SomeOtherWf"
+	}, 10*time.Second, 100*time.Millisecond)
+}
+
 func (s *SharedServerSuite) TestSchedule_Memo_Update() {
 	schedId, schedWfId, res := s.createSchedule("--memo", "bar=1")
 	s.NoError(res.Err)