From 01cc8776094f1f9834c7050abd76bba6067fbd08 Mon Sep 17 00:00:00 2001 From: lucas picollo Date: Tue, 18 Aug 2026 15:29:22 -0300 Subject: [PATCH] fix: accept an attachment scalar as either a string or a number #6 retyped `Attachment.Date` from int64 to string because ClickUp sends it as "1749381600000". Today the mirror image landed on the next field along: json: cannot unmarshal number into Go struct field Attachment.attachments.version of type string ClickUp is simply not consistent about which JSON scalar it uses here, and it can differ between the attachment endpoint and the copy embedded in a task response. Either Go scalar is therefore the wrong choice: whichever one is declared, some real response fails to unmarshal, and because the field sits inside every task payload the blast radius is `task get` AND `task update` for any task carrying an attachment -- the write applies server-side and still reports UNMARSHAL_ERROR, so a caller cannot tell it from a genuine failure. A queue-driven agent hit exactly that twice and both times spent the run on the CLI instead of the work. FlexString accepts both shapes and normalises to the string form, keeping the JSON output stable for consumers. It rejects anything that is not a scalar, so a real shape change still fails loudly rather than landing as "". Both fields use it; the regression test pins both shapes and the rejection. Co-Authored-By: Claude Opus 5 (1M context) --- .../api/attachment_date_regression_test.go | 57 +++++++++++++++---- internal/api/attachments.go | 39 ++++++++++--- 2 files changed, 77 insertions(+), 19 deletions(-) diff --git a/internal/api/attachment_date_regression_test.go b/internal/api/attachment_date_regression_test.go index dca9a38..f287d00 100644 --- a/internal/api/attachment_date_regression_test.go +++ b/internal/api/attachment_date_regression_test.go @@ -5,17 +5,52 @@ import ( "testing" ) -// TestAttachmentDateUnmarshalsStringFromAPI is a regression test: ClickUp's API -// returns an attachment "date" as a JSON string (e.g. "1749381600000"), not a -// number. When Date was typed int64, unmarshalling any task/attachment response -// failed with "cannot unmarshal string into Go struct field Attachment.date". -func TestAttachmentDateUnmarshalsStringFromAPI(t *testing.T) { - payload := []byte(`{"id":"abc123","version":"0","date":"1749381600000","title":"file.md","extension":"md"}`) - var a Attachment - if err := json.Unmarshal(payload, &a); err != nil { - t.Fatalf("failed to unmarshal attachment with string date: %v", err) +// ClickUp sends an attachment's scalar fields as either a JSON string or a JSON +// number, and not consistently per field or per endpoint: `date` arrived as +// "1749381600000" (#6) and `version` as 0 (2026-08-18). Either one typed as a +// plain Go scalar breaks unmarshalling of EVERY task response that carries an +// attachment, which takes out `task get` and `task update` for that task. Both +// fields are FlexString; this pins both shapes. +func TestAttachmentScalarsAcceptStringOrNumber(t *testing.T) { + for _, tc := range []struct { + name string + payload string + wantVersion FlexString + wantDate FlexString + }{ + { + name: "strings", + payload: `{"id":"abc123","version":"0","date":"1749381600000","title":"file.md"}`, + wantVersion: "0", + wantDate: "1749381600000", + }, + { + name: "numbers", + payload: `{"id":"abc123","version":0,"date":1749381600000,"title":"file.md"}`, + wantVersion: "0", + wantDate: "1749381600000", + }, + } { + t.Run(tc.name, func(t *testing.T) { + var a Attachment + if err := json.Unmarshal([]byte(tc.payload), &a); err != nil { + t.Fatalf("failed to unmarshal attachment: %v", err) + } + if a.Version != tc.wantVersion { + t.Fatalf("version: got %q, want %q", a.Version, tc.wantVersion) + } + if a.Date != tc.wantDate { + t.Fatalf("date: got %q, want %q", a.Date, tc.wantDate) + } + }) } - if a.Date != "1749381600000" { - t.Fatalf("unexpected date: %q", a.Date) +} + +// A field that is neither shape must fail loudly rather than land as an empty +// string that reads like a missing value downstream. +func TestAttachmentScalarRejectsNonScalar(t *testing.T) { + var a Attachment + if err := json.Unmarshal([]byte(`{"id":"abc","version":{"n":1}}`), &a); err == nil { + t.Fatal("expected an error for an object in a FlexString field") } } diff --git a/internal/api/attachments.go b/internal/api/attachments.go index 8be0dc5..ddda395 100644 --- a/internal/api/attachments.go +++ b/internal/api/attachments.go @@ -12,15 +12,38 @@ import ( "path/filepath" ) +// FlexString is a string that also accepts a JSON number. ClickUp is not +// consistent about which scalar it uses for a field: an attachment's `date` +// arrives as "1749381600000" while its `version` arrives as 0, and the same +// field can differ between the attachment endpoint and the copy embedded in a +// task response. Typing such a field as either Go scalar makes every response +// carrying an attachment fail to unmarshal, which took out `task get` and +// `task update` twice (#6, and again on 2026-08-18 with `version`). +type FlexString string + +func (f *FlexString) UnmarshalJSON(b []byte) error { + var s string + if err := json.Unmarshal(b, &s); err == nil { + *f = FlexString(s) + return nil + } + var n json.Number + if err := json.Unmarshal(b, &n); err != nil { + return fmt.Errorf("value is neither a string nor a number: %s", string(b)) + } + *f = FlexString(n.String()) + return nil +} + type Attachment struct { - ID string `json:"id"` - Version string `json:"version"` - Date string `json:"date"` - Title string `json:"title"` - Extension string `json:"extension"` - ThumbnailSmall string `json:"thumbnail_small"` - ThumbnailLarge string `json:"thumbnail_large"` - URL string `json:"url"` + ID string `json:"id"` + Version FlexString `json:"version"` + Date FlexString `json:"date"` + Title string `json:"title"` + Extension string `json:"extension"` + ThumbnailSmall string `json:"thumbnail_small"` + ThumbnailLarge string `json:"thumbnail_large"` + URL string `json:"url"` } func (c *Client) CreateTaskAttachment(ctx context.Context, taskID, filePath string, opts ...*TaskScopedOptions) (*Attachment, error) {