Follow-up from #435, where @NickJosevski observed this during review (#435 (review)). Recording it here so it is not rediscovered later. Not a regression: negative time spans were handled incorrectly before #435 and remain incorrect after it, in both directions.
FromTimeSpan (parsing)
The sign is not applied to the whole value. As of #435 a leading - only reaches the day component, and in the plain hh:mm:ss form it is dropped entirely:
FromTimeSpan("-1.02:03:04") = -21h56m56s // .NET means -(1d 2h 3m 4s) = -26h3m4s
FromTimeSpan("-00:05:00") = 5m0s // sign dropped entirely
.NET serialises a negative TimeSpan with a single leading minus that negates the entire value, e.g. -1.02:03:04.
ToTimeSpan (formatting)
A negative time.Duration produces a component-wise negative string that no .NET parser accepts:
ToTimeSpan(-26h3m4s) = "-1.-2:-3:-4"
ToTimeSpan predates #435 and was untouched by it, which suggests negative spans were never supported on either side.
Scope
Both pkg/machines/duration_formatter.go and pkg/machinepolicies/duration_formatter.go carry a copy of these functions, so a fix needs to land in both, mirroring what #435 did for the day and fractional-second components.
Practical impact
Likely low. All in-repo callers of FromTimeSpan are in UnmarshalJSON for machine policy, cleanup policy, and health check policy, and Octopus Server is not expected to emit negative durations for any of those fields. Worth fixing mainly so a round trip through the SDK cannot silently flip or corrupt a sign if one ever appears.
Follow-up from #435, where @NickJosevski observed this during review (#435 (review)). Recording it here so it is not rediscovered later. Not a regression: negative time spans were handled incorrectly before #435 and remain incorrect after it, in both directions.
FromTimeSpan (parsing)
The sign is not applied to the whole value. As of #435 a leading
-only reaches the day component, and in the plainhh:mm:ssform it is dropped entirely:.NET serialises a negative
TimeSpanwith a single leading minus that negates the entire value, e.g.-1.02:03:04.ToTimeSpan (formatting)
A negative
time.Durationproduces a component-wise negative string that no .NET parser accepts:ToTimeSpanpredates #435 and was untouched by it, which suggests negative spans were never supported on either side.Scope
Both
pkg/machines/duration_formatter.goandpkg/machinepolicies/duration_formatter.gocarry a copy of these functions, so a fix needs to land in both, mirroring what #435 did for the day and fractional-second components.Practical impact
Likely low. All in-repo callers of
FromTimeSpanare inUnmarshalJSONfor machine policy, cleanup policy, and health check policy, and Octopus Server is not expected to emit negative durations for any of those fields. Worth fixing mainly so a round trip through the SDK cannot silently flip or corrupt a sign if one ever appears.