This is ready, and I'm approving it: it's a small, display-only change, and I didn't find anything in it that needs another reviewer.
What changed: commit 4096053 follows Paul's suggestion and adds .UTC() to the four remaining BuildDate.Format("... UTC") calls in cli/commands/upgrade_helpers.go, at lines 134, 142, 167 and 168. The search I ran came back truncated, but every build-date format ending in a literal UTC that it returned now converts to UTC before formatting.
The new test: TestPrintVersionComparisonBuildDatesUTC gives the two timestamps opposite offsets (CDT −5 and JST +9), so leaving .UTC() off either one fails the test. The expected values are right:
Stdout swap in the test: the test temporarily replaces os.Stdout. That's safe here because nothing in cli/commands calls t.Parallel(). The output is also small enough to fit in the pipe buffer, so closing the writer before reading can't hang.
No new tests for two lines: the two changed lines in CheckIfUpgradeNeeded don't have their own test. They're the same one-token change as the lines that are tested, so I wouldn't hold the merge for it.
Verdict: ready
{
"owner": "mirendev",
"repo": "runtime",
"number": 1272,
"verdict": "ready",
"approve": true,
"event": "approve",
"summary": "This is ready, and I'm approving it: it's a small, display-only change, and I didn't find anything in it that needs another reviewer.\n\n**What changed:** commit `4096053` follows Paul's suggestion and adds `.UTC()` to the four remaining `BuildDate.Format(\"... UTC\")` calls in `cli/commands/upgrade_helpers.go`, at lines 134, 142, 167 and 168. The search I ran came back truncated, but every build-date format ending in a literal `UTC` that it returned now converts to UTC before formatting.\n\n**The new test:** `TestPrintVersionComparisonBuildDatesUTC` gives the two timestamps opposite offsets (CDT −5 and JST +9), so leaving `.UTC()` off either one fails the test. The expected values are right:\n- 12:48 CDT on the 14th is 17:48 UTC.\n- 04:10 JST on the 15th is 19:10 UTC on the 14th.\n\n**Stdout swap in the test:** the test temporarily replaces `os.Stdout`. That's safe here because nothing in `cli/commands` calls `t.Parallel()`. The output is also small enough to fit in the pipe buffer, so closing the writer before reading can't hang.\n\n**No new tests for two lines:** the two changed lines in `CheckIfUpgradeNeeded` don't have their own test. They're the same one-token change as the lines that are tested, so I wouldn't hold the merge for it.",
"posted_to_pr": true,
"head_sha": "4096053ed7976305d3c0de58c5eec290ec012a46"
}This is ready to merge. It's a one-line fix, and I traced it end to end.
Why the bug happened: fetchServerVersion (cli/commands/version_server.go:52) builds the server's BuildDate with standard.FromTimestamp. That function calls time.Unix(...), which returns the time in the CLI host's local zone. The old printBuildLines formatted that local wall-clock time and then added a literal UTC. So anyone running the CLI outside UTC saw the server build time shifted by their own offset.
Why the fix is right: calling .UTC() before Format in cli/commands/version.go:148 fixes the text output. The CLI line is unaffected, because version.GetInfo() already normalizes with t.UTC(). The --format json path doesn't go through printBuildLines. It still emits an RFC3339 timestamp with the local offset, which is still the correct instant.
Tests: the new test in cli/commands/version_test.go covers the three cases that matter:
Built: lineUsing a bare &Context{Stdout: &out} is safe, because Context.Printf only writes to c.Stdout.
Verdict: ready