fix(time): decode a millisecond field whichever type ClickUp sends

[isaacrowntree]

Fixes #27.

What was wrong

ClickUp is inconsistent about millisecond fields: time_spent comes back as the number 0 while nothing is tracked and as the string "2040000" once something is — both in the same response. Every millisecond field in the CLI was a Go scalar, so one tracked subtask aborted the decode of its whole parent:

$ clickup task view 86cay82n2
failed to fetch task 86cay82n2: decode response: json: cannot unmarshal string
into Go struct field Task.subtasks.time_spent of type int64

No partial output, no fallback — the task's name, status and every other subtask went with it.

The report noted the parent's own time_spent seemed to be handled already. It wasn't: Subtasks is []Task, so parent and child share the field. Viewing the tracked subtask directly only worked because the API sent that same value as a number when the task was the top-level object.

The fix

clickup.Millis decodes from a number or a string (and from null, an empty string, or an in-range float) and marshals back as a number. It now backs:

• Task.time_spent / Task.time_estimate — which covers subtasks, since they are Tasks too. This is the reported bug.
• Every millisecond field in a generated response type, via x-go-type in patch-v2-spec.jq, scoped to the time-entry paths. This caught two latent breaks of the same class: task time stop / task time running decoded duration as int and start as string, and stop is the worst place to fail — the timer is already stopped server-side, so the user gets an error and then "no timer is running" on retry.
• The task time list entry struct, so the timesheet total is arithmetic on a real integer rather than a re-parsed string.

Request bodies deliberately keep plain scalars: the CLI is the side sending them, and widening them silently drops the --duration flag the generated flag sets bind.

Output contract

Two deliberate changes, both pinned by tests:

Command	Field	Was	Now
task time list --json	duration, start, end	quoted string	number
task time stop/start/running --json	start, at	quoted string	number

The old shapes were internally inconsistent — a quoted start beside a numeric end in the same object — and could not decode at all when ClickUp swapped them. One numeric shape across the command group means a single jq filter works on all of it. The documented .duration | tonumber recipe still works.

view tasks --json keeps emitting time_spent/time_estimate for untracked tasks: the patch sets x-omitempty: false, because omitempty on an integer kind drops a zero before MarshalJSON is ever consulted.

Review

Twelve review passes over three rounds. Every finding that held up is fixed here, each with a test that was red first — the full accounting is in the comments below. The ones worth knowing about:

• view tasks --json had begun dropping zero-valued time fields (omitempty + int64 kind).
• Millis accepted Inf/NaN/overflow through its float fallback, and the int64 conversion is implementation-defined — 1e30 decoded to MaxInt64 on arm64 and MinInt64 on amd64.
• The timesheet total was silently short: a float duration printed as 1800000.0 and was dropped from the sum, so 1h + 30m reported Total: 1h across 2 entries.
• duration was patched spec-wide while the timestamps were path-scoped — backwards, since duration is the more generic word.
• --time-estimate collided with task edit's hand-written flag, safe only by registration order; pflag panics on a redefinition at startup.
• api/specs/clickup-v2.json had no prerequisite on the jq patch, so editing it left existing checkouts generating from stale rules with make api-gen reporting nothing to do.
• cmd/gen-api ignored x-go-type, so a $ref-resolved millis field would have been emitted as a rigid scalar — reopening clickup task view --json fails to decode any task whose subtask has tracked time #27 on the endpoint the patch had just fixed.
• CI installed oapi-codegen-exp@latest; make check regenerates and compares, which proves nothing if the generator floats. Pinned via make tools.

Tests

Written first, red before each fix — the type (number, string, null, empty, negative, float, garbage, out-of-range, the 2^63 boundary), clickup.Task decode, the issue's exact response, the list endpoint, task view and its --json shape, view tasks --json, task time list including the timesheet total, all three timer commands, the spec patch against both synthetic fixtures and the real pinned spec, and the gen-api type mapping.

Only the real API produces the mixed types, so smoke.sh gained the time-tracking path: log time on a subtask, list it back, read the parent, then the timer round trip. It skips the timer steps when the account already has one running — none of start/running/stop take an id, so they would otherwise displace a developer's real tracking — and registers both the timer and the logged entry with cleanup().

go build, go test ./..., go vet, gofmt and the docs check are clean after a full make api-clean api-gen, and smoke passes against the workspace. golangci-lint reports two pre-existing errcheck findings in internal/taskindex from bd748fb, unrelated to this branch.

🤖 Generated with Claude Code

https://claude.ai/code/session_018A2UDG7xce7xjTJU3HPLy3

[isaacrowntree]

Review pass on this branch turned up four things; all four are fixed in ed67612, each with a test that was red first.

1. The same defect one field over (medium). A time entry's start, end and at swing between string and number exactly as duration does — the spec's own stop-timer example shows "start":"1595289395842" beside "end":1595289452790, while the running-timer example makes all three strings. The generated code inherited the contradiction (Start string / End int / At int on stop; Start string / At string on current), and runTimeRunning parsed data.Start with ParseInt, so a numeric start failed the whole command.

Stop is the worst place for this: the timer is already stopped server-side by the time the CLI decodes the response, so the user gets an error and then, on retry, "no timer is running." Both forms now decode. The timestamp fix is scoped to time-entry paths by name — start and end mean other things elsewhere in the spec — with a test asserting a task's own start is left alone.

2. Timer stopped — logged (low). Swapping formatDuration for formatMillisDuration in the stop path quietly changed the zero case: the former says 0m, the latter renders anything non-positive as empty. ClickUp does return non-positive durations here. Reverted to formatDuration; TestTimeStop_ShortDurationStillReportsATime pins it.

3. components was wider than its own comment claimed (low). The patch narrowed millis fields to response schemas so request bodies keep the plain scalars their flag sets bind --duration to — then walked all of components, request bodies included. Inert today (this spec's components holds only securitySchemes), but it would have silently retyped --duration the moment upstream factored a time-entry request body out. Now scoped to components.schemas and components.responses.

4. smoke.sh assertions couldn't fire (low). Under set -euo pipefail, LOGGED=$(…) aborts the script the instant the CLI exits non-zero — so a decode failure, the exact regression those steps were added to catch, surfaced as a bare bash exit instead of the fail message. Now captured with the set +e / rc pattern the existing search_finds helper uses.

smoke.sh also gained the timer round trip (start → running → stop), since only the real API produces the mixed types. Full run green against the workspace.

[isaacrowntree]

Ten review passes over the branch. Seven findings held up and are fixed in 0cc0499, each with a test that failed first. The rest are recorded below with why they were not acted on.

Fixed

1. view tasks --json was dropping zero-valued time fields (the real regression). The generated fields carry omitempty, and encoding/json decides that from the reflect kind before MarshalJSON is consulted — so Millis(0) vanished where the Nullable[int] it replaced emitted 0. Reproduced: {"id":"t1","name":"Untracked","time_spent":0} marshalled to {"id":"t1","name":"Untracked"}, so --jq 'map(.time_spent) | add' returned null instead of a total. The patch now emits x-omitempty: false, which the codegen honours.

2. Millis accepted values int64 cannot hold. ParseInt's range error fell through to ParseFloat, which takes Inf, NaN and any magnitude — and converting an out-of-range float is implementation-defined in Go, so 1e30 decoded to MaxInt64 on arm64 and MinInt64 on amd64 from identical bytes. Now an error, boundary included: math.MaxInt64 is not representable as a float64, so the check is f < math.MaxInt64, pinned by its own test.

3. The timesheet total was silently short. flexibleInt keeps the raw token, so a float duration printed as 1800000.0 in the duration column and was dropped from the sum — two entries of 1h and 30m reported Total: 1h across 2 entries. Reproduced end-to-end before fixing. timeEntry uses clickup.Millis now, like the generated models for the sibling endpoints.

4. Two normalisations in one command group. Consequence of (3): duration/start/end are JSON numbers in task time list --json where they were quoted strings, matching the timer commands. Combined with the string→number flip on start/at from ed67612, this is a deliberate output-contract change — the old shapes were internally inconsistent (start a quoted string beside a numeric end in the same object) and could not decode at all when ClickUp swapped them. Both shapes are now pinned by tests rather than left to drift. The documented .duration | tonumber recipe still works.

5. duration was patched spec-wide while start/end/at were path-scoped. Backwards — duration is the more generic word. A future duration meaning seconds would have decoded clean and been wrong by 1000×. Both are scoped to the time-entry paths now.

6. Nothing asserted the patch fires on the real spec. Every millis test invented its own path, so a rename upstream would leave the suite green and the fix gone. Two tests now run jq against the pinned spec: every millisecond field on the timer and task endpoints must carry the flexible type, and no request body may.

7. --time-estimate and the stale-checkout trap. Narrowing the patch off request bodies made the codegen emit a scalar --time-estimate flag, colliding with task edit's hand-written one — safe only by registration order, and pflag panics at startup on a redefinition. It is in the skip list now. Separately, api/specs/clickup-v2.json had no prerequisite on the jq, so editing the patch left every existing checkout generating from stale rules with make api-gen reporting nothing to do; the target depends on the patch file now.

smoke.sh no longer displaces a real timer: it skips the round trip when the account already has one running (none of start/running/stop take an id — they act on whatever the account has), registers both the timer and the logged entry with cleanup(), keeps stderr in its failure messages, and asserts the subtask count instead of printing whatever it found. Full run green against the workspace.

Reported, not acted on

• Millis is stricter than the string it replaced on start/at. True: a non-numeric string there now fails the decode. But the alternative is the bug this PR exists to fix — those fields were string on one endpoint and int on another, and the CLI already parsed them as integers. An ISO-8601 start is hypothetical; the mixed int/string forms are in the spec's own examples.
• Millis collapses null into 0. Real, and unfixable without a pointer, which the codegen does not emit for x-go-type. Affects end: null on a running entry. Worth a follow-up if it bites.
• The decode error no longer names the field. Inherent to a custom json.Unmarshaler — encoding/json only decorates its own UnmarshalTypeError.
• formatMillisDuration / formatSprintDuration duplication. Collapsed the four int64→string→int64 round trips through formatDuration into one formatDurationMs core; left the cross-package duplicate alone as out of scope.
• taskindex errcheck failures block make lint. Confirmed pre-existing on main (bd748fb), not from this branch.

go build, go test ./..., go vet, gofmt and the docs check are clean after a full make api-clean api-gen.