From 5dbf56bb45a7d2db965b8ca8e82e10669f8d657a Mon Sep 17 00:00:00 2001 From: Ryan Lewis Date: Sat, 20 Jun 2026 21:03:12 +0100 Subject: [PATCH 1/3] feat(output): emit human-readable status string in JSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JSON output now renders task/project/checklist status as a string enum ("open" | "cancelled" | "completed") instead of the raw Things integer (0/2/3). The non-contiguous ints were pure implementation detail and easy to misread — a bare 3 reads as a rank, not "done". Introduces a model.Status named type with String/MarshalJSON, plus an UnmarshalJSON that accepts both the string name and the legacy integer so existing JSON round-trips and integer input still parse. Closes #105 --- internal/db/projects.go | 2 +- internal/model/model.go | 62 ++++++++++++++++++++++++++++++---- internal/model/model_test.go | 62 ++++++++++++++++++++++++++++++++++ internal/output/output.go | 4 +-- internal/output/output_test.go | 2 +- internal/output/style.go | 2 +- 6 files changed, 123 insertions(+), 11 deletions(-) diff --git a/internal/db/projects.go b/internal/db/projects.go index 5a80275..034a332 100644 --- a/internal/db/projects.go +++ b/internal/db/projects.go @@ -52,7 +52,7 @@ func (d *DB) ListProjects(areaFilter string, includeCompleted bool) ([]model.Pro return nil, fmt.Errorf("scanning project: %w", err) } if status.Valid { - p.Status = int(status.Int64) + p.Status = model.Status(status.Int64) } if tagsStr != "" { p.Tags = strings.Split(tagsStr, "\x1f") diff --git a/internal/model/model.go b/internal/model/model.go index 577c510..f7c53d9 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -10,15 +10,65 @@ const ( TypeTask = 0 TypeProject = 1 - StatusOpen = 0 - StatusCancelled = 2 - StatusCompleted = 3 + StatusOpen Status = 0 + StatusCancelled Status = 2 + StatusCompleted Status = 3 StartInbox = 0 StartAnytime = 1 StartSomeday = 2 ) +// Status is a Things3 task/project status. The underlying integers are the +// raw Things codes (0 = open, 2 = cancelled, 3 = completed — note there is no +// 1), but JSON renders the human-readable string so scripts and agents never +// have to decode the magic ints. +type Status int + +func (s Status) String() string { + switch s { + case StatusOpen: + return "open" + case StatusCancelled: + return "cancelled" + case StatusCompleted: + return "completed" + default: + return "unknown" + } +} + +// MarshalJSON renders the status as its string name ("open"/"cancelled"/ +// "completed") rather than the raw Things integer. +func (s Status) MarshalJSON() ([]byte, error) { + return json.Marshal(s.String()) +} + +// UnmarshalJSON accepts either the string name or the raw integer so JSON +// emitted by this tool round-trips and legacy integer input still parses. +func (s *Status) UnmarshalJSON(data []byte) error { + var str string + if err := json.Unmarshal(data, &str); err == nil { + switch str { + case "open": + *s = StatusOpen + case "cancelled": + *s = StatusCancelled + case "completed": + *s = StatusCompleted + default: + return fmt.Errorf("Status: unknown value %q", str) + } + return nil + } + var n int + if err := json.Unmarshal(data, &n); err != nil { + return fmt.Errorf("Status: %w", err) + } + *s = Status(n) + return nil +} + // ThingsDate is a bit-encoded date: year<<16 | month<<12 | day<<7. type ThingsDate int64 @@ -73,7 +123,7 @@ type Task struct { Title string `json:"title"` Notes string `json:"notes,omitempty"` Type int `json:"type"` - Status int `json:"status"` + Status Status `json:"status"` Start int `json:"start"` StartBucket int `json:"startBucket"` StartDate *ThingsDate `json:"startDate,omitempty"` @@ -95,7 +145,7 @@ type Task struct { type ChecklistItem struct { UUID string `json:"uuid"` Title string `json:"title"` - Status int `json:"status"` + Status Status `json:"status"` StopDate *time.Time `json:"stopDate,omitempty"` Index int `json:"index"` } @@ -103,7 +153,7 @@ type ChecklistItem struct { type Project struct { UUID string `json:"uuid"` Title string `json:"title"` - Status int `json:"status"` + Status Status `json:"status"` AreaUUID string `json:"areaUUID,omitempty"` AreaTitle string `json:"areaTitle,omitempty"` Tags []string `json:"tags,omitempty"` diff --git a/internal/model/model_test.go b/internal/model/model_test.go index 899751c..2a2f07f 100644 --- a/internal/model/model_test.go +++ b/internal/model/model_test.go @@ -135,3 +135,65 @@ func TestCoreDataEpochZero(t *testing.T) { t.Fatalf("CoreDataToTime(0) = %s, want %s", got, epoch) } } + +func TestStatusMarshalJSON(t *testing.T) { + cases := []struct { + status Status + want string + }{ + {StatusOpen, `"open"`}, + {StatusCancelled, `"cancelled"`}, + {StatusCompleted, `"completed"`}, + {Status(99), `"unknown"`}, + } + for _, tc := range cases { + got, err := json.Marshal(tc.status) + if err != nil { + t.Fatalf("Marshal(%d): %v", tc.status, err) + } + if string(got) != tc.want { + t.Errorf("Marshal(%d) = %s, want %s", tc.status, got, tc.want) + } + } +} + +func TestStatusUnmarshalJSON(t *testing.T) { + cases := []struct { + in string + want Status + }{ + {`"open"`, StatusOpen}, + {`"cancelled"`, StatusCancelled}, + {`"completed"`, StatusCompleted}, + {`0`, StatusOpen}, // legacy integer input + {`3`, StatusCompleted}, // legacy integer input + } + for _, tc := range cases { + var s Status + if err := json.Unmarshal([]byte(tc.in), &s); err != nil { + t.Fatalf("Unmarshal(%s): %v", tc.in, err) + } + if s != tc.want { + t.Errorf("Unmarshal(%s) = %d, want %d", tc.in, s, tc.want) + } + } + var s Status + if err := json.Unmarshal([]byte(`"bogus"`), &s); err == nil { + t.Error("Unmarshal of unknown string should error") + } +} + +func TestStatusRoundTripJSON(t *testing.T) { + in := Task{Title: "t", Status: StatusCompleted} + data, err := json.Marshal(in) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + var out Task + if err := json.Unmarshal(data, &out); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + if out.Status != StatusCompleted { + t.Errorf("round-trip status = %d, want %d", out.Status, StatusCompleted) + } +} diff --git a/internal/output/output.go b/internal/output/output.go index e86da40..e6f1732 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -296,7 +296,7 @@ func printTags(w io.Writer, tags []model.Tag) error { return nil } -func statusIcon(status int) string { +func statusIcon(status model.Status) string { switch status { case model.StatusOpen: return "[ ]" @@ -309,7 +309,7 @@ func statusIcon(status int) string { } } -func statusText(status int) string { +func statusText(status model.Status) string { switch status { case model.StatusOpen: return "Open" diff --git a/internal/output/output_test.go b/internal/output/output_test.go index c7ac82f..422a6de 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -255,7 +255,7 @@ func TestPrintFallbackJSON(t *testing.T) { func TestStatusHelpers(t *testing.T) { cases := []struct { - status int + status model.Status icon string text string }{ diff --git a/internal/output/style.go b/internal/output/style.go index ad64ca0..9983224 100644 --- a/internal/output/style.go +++ b/internal/output/style.go @@ -77,7 +77,7 @@ var ( // nowFn is overridable in tests. var nowFn = time.Now -func styledStatus(status int) string { +func styledStatus(status model.Status) string { icon := statusIcon(status) switch status { case model.StatusCompleted: From de7923c0c02c3256fbe427231036b6e0734b91be Mon Sep 17 00:00:00 2001 From: Ryan Lewis Date: Sat, 20 Jun 2026 21:10:03 +0100 Subject: [PATCH 2/3] refactor(model): make Status JSON round-trip lossless and symmetric Address code-review findings on the Status codec: - Single source of truth: String/MarshalJSON/UnmarshalJSON now share one statusNames map instead of three hand-maintained switches that could drift. - Lossless round-trip: an unrecognized raw Things code now marshals as its integer instead of a lossy "unknown" string that UnmarshalJSON rejected. - Honest errors: UnmarshalJSON dispatches on the JSON token type, so a malformed string token reports a string error rather than silently falling through to the integer branch and surfacing a misleading int-parse error. --- internal/model/model.go | 60 ++++++++++++++++++++---------------- internal/model/model_test.go | 38 ++++++++++++++--------- 2 files changed, 57 insertions(+), 41 deletions(-) diff --git a/internal/model/model.go b/internal/model/model.go index f7c53d9..1b67806 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -1,6 +1,7 @@ package model import ( + "bytes" "encoding/json" "fmt" "time" @@ -25,41 +26,48 @@ const ( // have to decode the magic ints. type Status int +// statusNames is the single source of truth for the name<->code mapping used +// by String, MarshalJSON, and UnmarshalJSON. +var statusNames = map[Status]string{ + StatusOpen: "open", + StatusCancelled: "cancelled", + StatusCompleted: "completed", +} + func (s Status) String() string { - switch s { - case StatusOpen: - return "open" - case StatusCancelled: - return "cancelled" - case StatusCompleted: - return "completed" - default: - return "unknown" + if name, ok := statusNames[s]; ok { + return name } + return "unknown" } -// MarshalJSON renders the status as its string name ("open"/"cancelled"/ -// "completed") rather than the raw Things integer. +// MarshalJSON renders a recognized status as its string name +// ("open"/"cancelled"/"completed"). An unrecognized raw Things code is +// preserved as its integer so the value round-trips losslessly rather than +// collapsing to a lossy "unknown" string. func (s Status) MarshalJSON() ([]byte, error) { - return json.Marshal(s.String()) + if name, ok := statusNames[s]; ok { + return json.Marshal(name) + } + return json.Marshal(int(s)) } -// UnmarshalJSON accepts either the string name or the raw integer so JSON -// emitted by this tool round-trips and legacy integer input still parses. +// UnmarshalJSON accepts either a status name or the raw Things integer, +// mirroring MarshalJSON so values round-trip. Names are matched strictly +// against the known set; integers are taken verbatim as the raw wire code. func (s *Status) UnmarshalJSON(data []byte) error { - var str string - if err := json.Unmarshal(data, &str); err == nil { - switch str { - case "open": - *s = StatusOpen - case "cancelled": - *s = StatusCancelled - case "completed": - *s = StatusCompleted - default: - return fmt.Errorf("Status: unknown value %q", str) + if trimmed := bytes.TrimSpace(data); len(trimmed) > 0 && trimmed[0] == '"' { + var name string + if err := json.Unmarshal(trimmed, &name); err != nil { + return fmt.Errorf("Status: %w", err) + } + for st, n := range statusNames { + if n == name { + *s = st + return nil + } } - return nil + return fmt.Errorf("Status: unknown value %q", name) } var n int if err := json.Unmarshal(data, &n); err != nil { diff --git a/internal/model/model_test.go b/internal/model/model_test.go index 2a2f07f..cda71d2 100644 --- a/internal/model/model_test.go +++ b/internal/model/model_test.go @@ -144,7 +144,7 @@ func TestStatusMarshalJSON(t *testing.T) { {StatusOpen, `"open"`}, {StatusCancelled, `"cancelled"`}, {StatusCompleted, `"completed"`}, - {Status(99), `"unknown"`}, + {Status(99), `99`}, // unrecognized code preserved as its raw int } for _, tc := range cases { got, err := json.Marshal(tc.status) @@ -167,6 +167,7 @@ func TestStatusUnmarshalJSON(t *testing.T) { {`"completed"`, StatusCompleted}, {`0`, StatusOpen}, // legacy integer input {`3`, StatusCompleted}, // legacy integer input + {`99`, Status(99)}, // unrecognized raw code taken verbatim } for _, tc := range cases { var s Status @@ -177,23 +178,30 @@ func TestStatusUnmarshalJSON(t *testing.T) { t.Errorf("Unmarshal(%s) = %d, want %d", tc.in, s, tc.want) } } - var s Status - if err := json.Unmarshal([]byte(`"bogus"`), &s); err == nil { - t.Error("Unmarshal of unknown string should error") + // Unknown string names are rejected, but a malformed JSON token must not be + // silently funnelled into the integer branch. + for _, bad := range []string{`"bogus"`, `{}`, `[1]`} { + var s Status + if err := json.Unmarshal([]byte(bad), &s); err == nil { + t.Errorf("Unmarshal(%s) succeeded, want error", bad) + } } } func TestStatusRoundTripJSON(t *testing.T) { - in := Task{Title: "t", Status: StatusCompleted} - data, err := json.Marshal(in) - if err != nil { - t.Fatalf("Marshal: %v", err) - } - var out Task - if err := json.Unmarshal(data, &out); err != nil { - t.Fatalf("Unmarshal: %v", err) - } - if out.Status != StatusCompleted { - t.Errorf("round-trip status = %d, want %d", out.Status, StatusCompleted) + // Both a recognized status and an unrecognized raw code must round-trip. + for _, want := range []Status{StatusCompleted, Status(99)} { + in := Task{Title: "t", Status: want} + data, err := json.Marshal(in) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + var out Task + if err := json.Unmarshal(data, &out); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + if out.Status != want { + t.Errorf("round-trip status = %d, want %d", out.Status, want) + } } } From 7f1888a8893f48f2ab1cbd2ed081e29f1dd2466a Mon Sep 17 00:00:00 2001 From: Ryan Lewis Date: Mon, 22 Jun 2026 23:14:50 +0100 Subject: [PATCH 3/3] fix(model): treat JSON null as no-op in Status codec; simplify dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address a second code-review pass on the Status codec: - Correctness: UnmarshalJSON silently coerced a JSON null into Status(0) ("open"), clobbering a pre-set status. Per the json.Unmarshaler convention, null is now a no-op that leaves the existing value untouched. - Reuse: replace the manual bytes.TrimSpace + leading-quote sniff (and its sole-purpose bytes import) with the stdlib idiom — try the string name, fall back to the raw integer on a *json.UnmarshalTypeError, surface other errors as-is. Same accept-string-or-legacy-int behavior, less hand-rolled. - Convention: document the JSON status string enum in internal/skill/SKILL.md (CLAUDE.md requires SKILL.md track output-surface changes). - Tests: add the legacy cancelled (2) integer-decode case, a null no-op case, and StatusCancelled to the round-trip. --- internal/model/model.go | 41 +++++++++++++++++++++++------------- internal/model/model_test.go | 12 ++++++++++- internal/skill/SKILL.md | 2 ++ 3 files changed, 39 insertions(+), 16 deletions(-) diff --git a/internal/model/model.go b/internal/model/model.go index 1b67806..4b42237 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -1,8 +1,8 @@ package model import ( - "bytes" "encoding/json" + "errors" "fmt" "time" ) @@ -56,25 +56,36 @@ func (s Status) MarshalJSON() ([]byte, error) { // mirroring MarshalJSON so values round-trip. Names are matched strictly // against the known set; integers are taken verbatim as the raw wire code. func (s *Status) UnmarshalJSON(data []byte) error { - if trimmed := bytes.TrimSpace(data); len(trimmed) > 0 && trimmed[0] == '"' { - var name string - if err := json.Unmarshal(trimmed, &name); err != nil { + // Per the json.Unmarshaler convention, a JSON null is a no-op: leave the + // existing value untouched rather than silently coercing it to Status(0) + // ("open"). + if string(data) == "null" { + return nil + } + // Try the string name first; on a type mismatch fall back to the raw + // integer so both the emitted string form and the legacy integer decode. A + // non-type error (malformed JSON) is surfaced as-is rather than retried as + // an int. + var name string + if err := json.Unmarshal(data, &name); err != nil { + var typeErr *json.UnmarshalTypeError + if !errors.As(err, &typeErr) { return fmt.Errorf("Status: %w", err) } - for st, n := range statusNames { - if n == name { - *s = st - return nil - } + var n int + if err := json.Unmarshal(data, &n); err != nil { + return fmt.Errorf("Status: %w", err) } - return fmt.Errorf("Status: unknown value %q", name) + *s = Status(n) + return nil } - var n int - if err := json.Unmarshal(data, &n); err != nil { - return fmt.Errorf("Status: %w", err) + for st, n := range statusNames { + if n == name { + *s = st + return nil + } } - *s = Status(n) - return nil + return fmt.Errorf("Status: unknown value %q", name) } // ThingsDate is a bit-encoded date: year<<16 | month<<12 | day<<7. diff --git a/internal/model/model_test.go b/internal/model/model_test.go index cda71d2..130d233 100644 --- a/internal/model/model_test.go +++ b/internal/model/model_test.go @@ -166,6 +166,7 @@ func TestStatusUnmarshalJSON(t *testing.T) { {`"cancelled"`, StatusCancelled}, {`"completed"`, StatusCompleted}, {`0`, StatusOpen}, // legacy integer input + {`2`, StatusCancelled}, // legacy integer input (the non-obvious code) {`3`, StatusCompleted}, // legacy integer input {`99`, Status(99)}, // unrecognized raw code taken verbatim } @@ -178,6 +179,15 @@ func TestStatusUnmarshalJSON(t *testing.T) { t.Errorf("Unmarshal(%s) = %d, want %d", tc.in, s, tc.want) } } + // A JSON null is a no-op: it must leave the existing value untouched rather + // than silently coercing it to Status(0) ("open"). + pre := StatusCompleted + if err := json.Unmarshal([]byte(`null`), &pre); err != nil { + t.Fatalf("Unmarshal(null): %v", err) + } + if pre != StatusCompleted { + t.Errorf("Unmarshal(null) = %d, want %d (unchanged)", pre, StatusCompleted) + } // Unknown string names are rejected, but a malformed JSON token must not be // silently funnelled into the integer branch. for _, bad := range []string{`"bogus"`, `{}`, `[1]`} { @@ -190,7 +200,7 @@ func TestStatusUnmarshalJSON(t *testing.T) { func TestStatusRoundTripJSON(t *testing.T) { // Both a recognized status and an unrecognized raw code must round-trip. - for _, want := range []Status{StatusCompleted, Status(99)} { + for _, want := range []Status{StatusCancelled, StatusCompleted, Status(99)} { in := Task{Title: "t", Status: want} data, err := json.Marshal(in) if err != nil { diff --git a/internal/skill/SKILL.md b/internal/skill/SKILL.md index 2eaed88..ae8ff24 100644 --- a/internal/skill/SKILL.md +++ b/internal/skill/SKILL.md @@ -13,6 +13,8 @@ today, upcoming, projects, or areas on macOS. Most commands accept `--json` / `-j`. Prefer it when parsing output. +In JSON, `status` is a string enum — `"open"`, `"cancelled"`, or `"completed"` (not the raw Things integer) — on tasks, projects, and checklist items. Filter with e.g. `jq 'select(.status=="open")'`. + Human output is styled with colors and aligned columns. Color auto-disables when piping or when `NO_COLOR` is set. Override with `--color=always|never` (default `auto`). JSON output is unaffected. ## Core commands