From 3695d9a06097000c7975eea02a53426cd1357043 Mon Sep 17 00:00:00 2001 From: Arnon Rotem-Gal-Oz Date: Tue, 8 Sep 2026 13:44:03 +0300 Subject: [PATCH 1/6] ci: run CI on pushes to master MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workflow watched `main`, which this repo does not have, so push builds never ran — only pull_request ones did. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 75ead53..e276c30 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,7 +2,7 @@ name: CI on: push: - branches: [main] + branches: [master] pull_request: jobs: From ab2da002e3028e82f87b1560983ea4d20593e429 Mon Sep 17 00:00:00 2001 From: Arnon Rotem-Gal-Oz Date: Tue, 8 Sep 2026 13:44:06 +0300 Subject: [PATCH 2/6] fix(cli): report malformed invocations as USAGE, not INTERNAL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mcli is driven by LLM agents in steady state, where INTERNAL reads as "mcli broke, retry" while USAGE reads as "your call was wrong, fix it". Cobra's validation failures arrived unclassified and exited 5, inviting agents to retry calls that can never succeed. Flag *parse* failures now route through a FlagErrorFunc, and the checks cobra runs after parsing — which bypass that func entirely — are matched on their message markers: argument counts, flag groups, and MarkFlagRequired violations. The last is the most common malformed call mcli sees, since nearly every item and column command requires --board. Namespace commands also reject unknown subcommands instead of printing help and exiting 0, so `mcli item lst` no longer looks like a success. Adds errs.AllCodes so docs can be verified against the real set of codes, and drops CodeConflict, which was specified but never emitted. Co-Authored-By: Claude Opus 5 --- internal/cli/root.go | 64 ++++++++++++++++++++++++++++---- internal/cli/root_test.go | 77 +++++++++++++++++++++++++++++++++++++++ internal/errs/errs.go | 18 ++++++++- 3 files changed, 149 insertions(+), 10 deletions(-) diff --git a/internal/cli/root.go b/internal/cli/root.go index 75c7656..d58a9dd 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -67,6 +67,9 @@ func init() { requireKnownSubcommands(rootCmd) + // Subcommands inherit this: cobra's FlagErrorFunc() walks up to the parent. + rootCmd.SetFlagErrorFunc(usageFlagError) + apischema.SetConfigDir(resolveConfigDir()) } @@ -74,6 +77,13 @@ func init() { // the user (or an LLM) at the command index. const unknownCommandHint = "Run 'mcli help' to see available commands." +// usageFlagError classifies a flag parse failure (unknown flag, missing value, bad +// value) as a usage error. Cobra returns these as plain errors, which would otherwise +// reach the exit-code translator unclassified and report INTERNAL. +func usageFlagError(cmd *cobra.Command, err error) error { + return errs.Usage("%s\nRun '%s --help' for usage.", err.Error(), cmd.CommandPath()) +} + // requireKnownSubcommands makes every namespace command (one with subcommands // but no action of its own) reject unknown subcommands. Cobra's default shows // the namespace's help and exits 0, so a typo like `mcli item lst` looks like a @@ -102,18 +112,56 @@ func Execute(ctx context.Context) error { return friendlyUnknownCommand(rootCmd.Execute()) } -// friendlyUnknownCommand rewrites cobra's bare "unknown command …" error (raised -// for an unknown top-level command) into a usage error that points at -// `mcli help`. This gives a typo an actionable message and exit code 1 (usage) -// instead of a generic exit 5, while preserving any "Did you mean …?" suggestion -// cobra already put in the message. Unknown subcommands are handled in -// requireKnownSubcommands and already carry the hint, so they don't match here. +// cobraArgErrorMarkers identify cobra's argument-count validation failures +// (cobra.ExactArgs and friends). Cobra builds these with fmt.Errorf and exposes no +// sentinel, so matching the message is the only hook available. The markers are the +// invariant middles of those messages rather than their prefixes, which vary +// ("accepts", "accepts at most", "accepts between … and …"). +var cobraArgErrorMarkers = []string{ + " arg(s), received ", // accepts N / at most N / between N and M + " arg(s), only received ", // requires at least N +} + +// cobraFlagGroupMarker identifies all three of cobra's flag-group validation +// failures (MarkFlagsMutuallyExclusive, RequiredTogether, OneRequired). +const cobraFlagGroupMarker = "flags in the group [" + +// cobraRequiredFlagMarker identifies MarkFlagRequired violations, e.g. +// `required flag(s) "board" not set`. Cobra raises these from ValidateRequiredFlags +// after parsing succeeds, so they bypass the FlagErrorFunc entirely. This is the +// most common malformed call mcli sees — nearly every item and column command +// requires --board — which makes it the worst one to report as INTERNAL. +const cobraRequiredFlagMarker = "required flag(s) " + +// friendlyUnknownCommand reclassifies cobra's own validation failures as usage +// errors so they exit 1 instead of a generic 5. +// +// This matters more than it looks: mcli is driven by LLM agents in steady state, and +// INTERNAL reads as "mcli broke, try again" while USAGE reads as "your call was +// wrong, fix it". Leaving a malformed invocation on exit 5 invites an agent to retry +// a call that can never succeed. Flag *parse* errors are handled by the FlagErrorFunc +// set in init; this covers the checks cobra runs after parsing, which do not route +// through it. +// +// The "unknown command" branch also keeps cobra's "Did you mean …?" suggestion. +// Unknown subcommands are handled in requireKnownSubcommands and already carry the +// hint, so they do not reach here. func friendlyUnknownCommand(err error) error { if err == nil { return nil } - if strings.HasPrefix(err.Error(), "unknown command ") { - return errs.Usage("%s\n%s", err.Error(), unknownCommandHint) + msg := err.Error() + + if strings.HasPrefix(msg, "unknown command ") { + return errs.Usage("%s\n%s", msg, unknownCommandHint) + } + if strings.Contains(msg, cobraFlagGroupMarker) || strings.HasPrefix(msg, cobraRequiredFlagMarker) { + return errs.Usage("%s", msg) + } + for _, marker := range cobraArgErrorMarkers { + if strings.Contains(msg, marker) { + return errs.Usage("%s", msg) + } } return err } diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 4ca1e0c..3980ff9 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -42,6 +42,83 @@ func TestFriendlyUnknownCommand_PassesThroughUnrelated(t *testing.T) { } } +// newCobraValidationTree builds a command whose args and flag groups are validated by +// cobra itself, wired the way rootCmd is. +func newCobraValidationTree(args cobra.PositionalArgs, requiredFlags ...string) *cobra.Command { + root := &cobra.Command{Use: "mcli", SilenceErrors: true, SilenceUsage: true} + root.SetFlagErrorFunc(usageFlagError) + + leaf := &cobra.Command{ + Use: "leaf", + Args: args, + RunE: func(*cobra.Command, []string) error { return nil }, + } + leaf.Flags().String("date", "", "") + leaf.Flags().String("due", "", "") + leaf.Flags().String("board", "", "") + leaf.MarkFlagsMutuallyExclusive("date", "due") + for _, name := range requiredFlags { + if err := leaf.MarkFlagRequired(name); err != nil { + panic(err) + } + } + + root.AddCommand(leaf) + return root +} + +// TestCobraValidationErrorsAreUsage drives real cobra rather than asserting on +// hand-written strings: the classifier matches cobra's message text, so a cobra +// upgrade that rewords one must fail here instead of silently regressing these +// invocations to INTERNAL (exit 5), which tells an agent to retry a call that can +// never succeed. +func TestCobraValidationErrorsAreUsage(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + args cobra.PositionalArgs + argv []string + required []string + }{ + {"exact args, too few", cobra.ExactArgs(1), []string{"leaf"}, nil}, + {"exact args, too many", cobra.ExactArgs(1), []string{"leaf", "a", "b"}, nil}, + {"at most", cobra.MaximumNArgs(1), []string{"leaf", "a", "b"}, nil}, + {"at least", cobra.MinimumNArgs(2), []string{"leaf", "a"}, nil}, + {"between", cobra.RangeArgs(2, 3), []string{"leaf", "a"}, nil}, + {"mutually exclusive flags", cobra.NoArgs, []string{"leaf", "--date", "x", "--due", "y"}, nil}, + {"unknown flag", cobra.NoArgs, []string{"leaf", "--bogus"}, nil}, + {"flag missing its value", cobra.NoArgs, []string{"leaf", "--board"}, nil}, + // Verified live: `mcli item update --status Done` (no --board) reported + // INTERNAL/5 until cobraRequiredFlagMarker was added. Nearly every item and + // column command requires --board, so this is the most-hit malformed call. + {"required flag not set", cobra.NoArgs, []string{"leaf"}, []string{"board"}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + root := newCobraValidationTree(tc.args, tc.required...) + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + root.SetArgs(tc.argv) + + err := friendlyUnknownCommand(root.Execute()) + if err == nil { + t.Fatal("expected an error") + } + e, ok := errors.AsType[*errs.Error](err) + if !ok { + t.Fatalf("expected *errs.Error, got %T (%v)", err, err) + } + if e.Code != errs.CodeUsage { + t.Errorf("expected USAGE (exit 1), got %s: %v", e.Code, err) + } + }) + } +} + // newNamespaceTree builds a minimal root → namespace → leaf tree mirroring the // real command layout (a namespace with no action of its own) and applies // requireKnownSubcommands to it. diff --git a/internal/errs/errs.go b/internal/errs/errs.go index 597a5d2..e082c58 100644 --- a/internal/errs/errs.go +++ b/internal/errs/errs.go @@ -20,8 +20,6 @@ const ( CodeRateLimited Code = "RATE_LIMITED" // CodeNotFound indicates a resource was not found. CodeNotFound Code = "NOT_FOUND" - // CodeConflict indicates a resource conflict. - CodeConflict Code = "CONFLICT" // CodeInternal indicates an unexpected internal error. CodeInternal Code = "INTERNAL" // CodeDaemonRequired indicates the mcli daemon is not running but is needed. @@ -30,6 +28,22 @@ const ( CodeInterrupted Code = "INTERRUPTED" ) +// AllCodes lists every Code mcli can report. It exists so documentation can be +// verified against the real set of codes: adding a Code here without also +// documenting it (see internal/cli skill doc) fails the test suite. +func AllCodes() []Code { + return []Code{ + CodeUsage, + CodeAPI, + CodeNotFound, + CodeAuth, + CodeRateLimited, + CodeInternal, + CodeDaemonRequired, + CodeInterrupted, + } +} + // Error is a structured error value carrying a Code, a human message, and an // optional wrapped cause. type Error struct { From 18e5935cfb8ef0a2430568b1d7e736dc952e87e5 Mon Sep 17 00:00:00 2001 From: Arnon Rotem-Gal-Oz Date: Tue, 8 Sep 2026 13:44:09 +0300 Subject: [PATCH 3/6] feat(item): typed column shorthands and stdin batch writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writing an item previously meant looking up a column ID and hand-encoding monday's raw column JSON. Shorthands (--status, --date/--due, --number, --text, --checkbox) address the board's single column of that type and are validated before anything is sent. Where a board has 0 or 2+ columns of a type, the command fails with USAGE and names the candidates rather than guessing. Passing `-` in place of --name/ reads rows from stdin (JSON array or one object per line, max 500) and writes them in one request against monday's per-minute complexity budget instead of N. Every row is validated up front, so a bad label fails the batch without a partial write, and the response reports errors[].index so only the failed rows are retried — item creation has no dedupe key, so re-sending the whole batch would duplicate. --dry-run prints the exact column_values that would be sent. Co-Authored-By: Claude Opus 5 --- internal/api/items/columns/columns.go | 11 +- internal/api/items/columns/encode.go | 218 ++++++ internal/api/items/columns/encode_test.go | 177 +++++ internal/cli/item.go | 349 ++++++--- internal/cli/item_batch.go | 565 +++++++++++++ internal/cli/item_batch_test.go | 914 ++++++++++++++++++++++ internal/cli/item_shorthand.go | 230 ++++++ internal/cli/item_shorthand_test.go | 238 ++++++ 8 files changed, 2593 insertions(+), 109 deletions(-) create mode 100644 internal/api/items/columns/encode.go create mode 100644 internal/api/items/columns/encode_test.go create mode 100644 internal/cli/item_batch.go create mode 100644 internal/cli/item_batch_test.go create mode 100644 internal/cli/item_shorthand.go create mode 100644 internal/cli/item_shorthand_test.go diff --git a/internal/api/items/columns/columns.go b/internal/api/items/columns/columns.go index f39d302..ef2ade8 100644 --- a/internal/api/items/columns/columns.go +++ b/internal/api/items/columns/columns.go @@ -1,7 +1,12 @@ -// Package columns decodes monday.com column values from their raw JSON representation -// into clean, typed Go values suitable for JSON output. +// Package columns converts monday.com column values between their raw JSON wire +// representation and clean Go values. // -// The write path is JSON-passthrough (handled elsewhere). This package is read-path only. +// Decode (this file) is the read path: raw column JSON → typed value for output. +// Encode (encode.go) is the write path for the CLI's typed shorthands: a scalar like +// "Done" or "2026-05-10" → the wire JSON monday expects. The two share one set of +// column types on purpose, so a shape documented on the read side is the shape +// written on the write side. Column types with no encoder are still written as raw +// JSON by the caller (mcli's --col escape hatch). package columns import ( diff --git a/internal/api/items/columns/encode.go b/internal/api/items/columns/encode.go new file mode 100644 index 0000000..b3af9a5 --- /dev/null +++ b/internal/api/items/columns/encode.go @@ -0,0 +1,218 @@ +package columns + +import ( + "encoding/json" + "sort" + "strconv" + "strings" + "time" + + "github.com/mondaycom/mcli/internal/errs" +) + +// encoderFn builds the monday wire JSON for one column type from a human-friendly +// input string. settingsStr is the column's settings_str, used for validation. +type encoderFn func(settingsStr, input string) (json.RawMessage, error) + +// encoders maps monday column types to write-side encoders. Every key here must +// also have a read-side decoder in registry, so the two halves of a column type +// cannot drift apart (enforced by TestEncoders_HaveDecoders). +// +// This set is deliberately small: it covers the types whose wire shape is +// unambiguous from a single scalar. Everything else is written through the raw +// escape hatch, where the caller supplies the JSON. +var encoders = map[string]encoderFn{ + "text": encodeText, + "status": encodeStatus, + "date": encodeDate, + "numbers": encodeNumbers, + "checkbox": encodeCheckbox, +} + +// EncodableTypes returns the column types Encode supports, sorted. +func EncodableTypes() []string { + out := make([]string, 0, len(encoders)) + for t := range encoders { + out = append(out, t) + } + sort.Strings(out) + return out +} + +// Encode builds the monday wire JSON for a column write from a human-friendly value. +// +// columnType is the column's type as monday reports it; settingsStr is the column's +// settings_str; input is the caller's value (e.g. "Done", "2026-05-10", "true"). +// +// Validation is the point, not a bonus: every mcli item mutation sends +// create_labels_if_missing, so an unvalidated status typo silently creates a new +// label on the board instead of failing. Encode rejects values the column cannot +// hold, with errs.Usage, before any request is built. +func Encode(columnType, settingsStr, input string) (json.RawMessage, error) { + fn, ok := encoders[columnType] + if !ok { + return nil, errs.Usage("no shorthand for column type %q: use --col =", columnType) + } + return fn(settingsStr, input) +} + +// encodeText writes a "text" column. Wire shape: a bare JSON string. +func encodeText(_ string, input string) (json.RawMessage, error) { + b, err := json.Marshal(input) + if err != nil { + return nil, errs.Usage("text: %v", err) + } + return b, nil +} + +// encodeStatus writes a "status" column. Wire shape: {"label":"Done"}. +// The label is matched against the column's configured labels — exactly first, then +// case-insensitively, adopting the board's own casing on a case-insensitive hit. +func encodeStatus(settingsStr, input string) (json.RawMessage, error) { + label := strings.TrimSpace(input) + if label == "" { + return nil, errs.Usage("status: label must not be empty") + } + + // An unparseable or absent settings_str means we cannot validate. Write anyway + // rather than blocking the caller on metadata we failed to read. + if labels := statusLabels(settingsStr); len(labels) > 0 { + match, ok := matchLabel(labels, label) + if !ok { + return nil, errs.Usage("status: %q is not a label on this column; available: %s", + label, strings.Join(labels, ", ")) + } + label = match + } + + return json.Marshal(map[string]string{"label": label}) +} + +// statusLabels returns a status column's labels in index order. It handles both +// shapes monday uses: the index→name object and the dropdown-style array. +func statusLabels(settingsStr string) []string { + if settingsStr == "" { + return nil + } + + var asObject struct { + Labels map[string]string `json:"labels"` + } + if err := json.Unmarshal([]byte(settingsStr), &asObject); err == nil && len(asObject.Labels) > 0 { + keys := make([]string, 0, len(asObject.Labels)) + for k := range asObject.Labels { + keys = append(keys, k) + } + sort.Slice(keys, func(i, j int) bool { + ni, erri := strconv.Atoi(keys[i]) + nj, errj := strconv.Atoi(keys[j]) + if erri == nil && errj == nil { + return ni < nj + } + return keys[i] < keys[j] + }) + out := make([]string, 0, len(keys)) + for _, k := range keys { + if name := asObject.Labels[k]; name != "" { + out = append(out, name) + } + } + return out + } + + var asArray dropdownSettings + if err := json.Unmarshal([]byte(settingsStr), &asArray); err == nil { + out := make([]string, 0, len(asArray.Labels)) + for _, l := range asArray.Labels { + if l.Name != "" { + out = append(out, l.Name) + } + } + return out + } + + return nil +} + +// matchLabel finds want among labels, preferring an exact match and falling back to +// a case-insensitive one. It returns the board's spelling, not the caller's. +func matchLabel(labels []string, want string) (string, bool) { + for _, l := range labels { + if l == want { + return l, true + } + } + for _, l := range labels { + if strings.EqualFold(l, want) { + return l, true + } + } + return "", false +} + +// dateLayouts are the input forms encodeDate accepts, most specific first. +var dateLayouts = []string{ + time.RFC3339, + "2006-01-02T15:04:05", + "2006-01-02T15:04", + "2006-01-02 15:04:05", + "2006-01-02 15:04", + "2006-01-02", +} + +// encodeDate writes a "date" column. Wire shape: {"date":"2026-05-10"}, plus +// {"time":"14:30:00"} when the input carries a time. monday stores date-column +// times in UTC, so a zoned input is converted. +func encodeDate(_ string, input string) (json.RawMessage, error) { + s := strings.TrimSpace(input) + if s == "" { + return nil, errs.Usage("date: value must not be empty") + } + + for _, layout := range dateLayouts { + t, err := time.Parse(layout, s) + if err != nil { + continue + } + if layout == "2006-01-02" { + return json.Marshal(map[string]string{"date": t.Format("2006-01-02")}) + } + t = t.UTC() + return json.Marshal(map[string]string{ + "date": t.Format("2006-01-02"), + "time": t.Format("15:04:05"), + }) + } + + return nil, errs.Usage("date: %q is not a date; use 2026-05-10, 2026-05-10T14:30, or an RFC3339 timestamp", s) +} + +// encodeNumbers writes a "numbers" column. Wire shape: a JSON string holding the +// number, e.g. "42". The caller's own formatting is preserved once it parses. +func encodeNumbers(_ string, input string) (json.RawMessage, error) { + s := strings.TrimSpace(input) + if s == "" { + // An empty numbers column is written as an empty string, which clears it. + return json.RawMessage(`""`), nil + } + if _, err := strconv.ParseFloat(s, 64); err != nil { + return nil, errs.Usage("number: %q is not numeric", s) + } + return json.Marshal(s) +} + +// encodeCheckbox writes a "checkbox" column. Wire shape: {"checked":"true"}. +func encodeCheckbox(_ string, input string) (json.RawMessage, error) { + s := strings.ToLower(strings.TrimSpace(input)) + switch s { + case "yes", "y": + s = "true" + case "no", "n": + s = "false" + } + b, err := strconv.ParseBool(s) + if err != nil { + return nil, errs.Usage("checkbox: %q is not a boolean; use true or false", input) + } + return json.Marshal(map[string]string{"checked": strconv.FormatBool(b)}) +} diff --git a/internal/api/items/columns/encode_test.go b/internal/api/items/columns/encode_test.go new file mode 100644 index 0000000..f6375cc --- /dev/null +++ b/internal/api/items/columns/encode_test.go @@ -0,0 +1,177 @@ +package columns + +import ( + "encoding/json" + "errors" + "strings" + "testing" + + "github.com/mondaycom/mcli/internal/errs" +) + +// codeOf returns an error's errs code, or "" if it is not an *errs.Error. +func codeOf(err error) errs.Code { + if e, ok := errors.AsType[*errs.Error](err); ok { + return e.Code + } + return "" +} + +// statusSettingsObject is the index→name form monday uses for status columns. +const statusSettingsObject = `{"labels":{"0":"Not Started","1":"Working on it","2":"Done"}}` + +// statusSettingsArray is the id/name array form some columns report instead. +const statusSettingsArray = `{"labels":[{"id":1,"name":"Open"},{"id":2,"name":"Closed"}]}` + +func TestEncode_ok(t *testing.T) { + tests := []struct { + name string + columnType string + settings string + input string + want string + }{ + {name: "text", columnType: "text", input: "Follow up", want: `"Follow up"`}, + {name: "text empty clears", columnType: "text", input: "", want: `""`}, + {name: "status exact", columnType: "status", settings: statusSettingsObject, input: "Done", want: `{"label":"Done"}`}, + { + name: "status case-insensitive adopts board casing", columnType: "status", + settings: statusSettingsObject, input: "done", want: `{"label":"Done"}`, + }, + {name: "status array settings", columnType: "status", settings: statusSettingsArray, input: "closed", want: `{"label":"Closed"}`}, + {name: "status unvalidated without settings", columnType: "status", input: "Whatever", want: `{"label":"Whatever"}`}, + {name: "date only", columnType: "date", input: "2026-05-10", want: `{"date":"2026-05-10"}`}, + {name: "date and time", columnType: "date", input: "2026-05-10T14:30", want: `{"date":"2026-05-10","time":"14:30:00"}`}, + {name: "date space separated", columnType: "date", input: "2026-05-10 14:30:05", want: `{"date":"2026-05-10","time":"14:30:05"}`}, + { + name: "rfc3339 converts to utc", columnType: "date", + input: "2026-05-10T14:30:00+03:00", want: `{"date":"2026-05-10","time":"11:30:00"}`, + }, + {name: "numbers int", columnType: "numbers", input: "42", want: `"42"`}, + {name: "numbers float", columnType: "numbers", input: "42.5", want: `"42.5"`}, + {name: "numbers negative", columnType: "numbers", input: "-7", want: `"-7"`}, + {name: "numbers empty clears", columnType: "numbers", input: "", want: `""`}, + {name: "checkbox true", columnType: "checkbox", input: "true", want: `{"checked":"true"}`}, + {name: "checkbox yes", columnType: "checkbox", input: "yes", want: `{"checked":"true"}`}, + {name: "checkbox n", columnType: "checkbox", input: "n", want: `{"checked":"false"}`}, + {name: "checkbox 0", columnType: "checkbox", input: "0", want: `{"checked":"false"}`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Encode(tt.columnType, tt.settings, tt.input) + if err != nil { + t.Fatalf("Encode(%q, %q) error: %v", tt.columnType, tt.input, err) + } + if string(got) != tt.want { + t.Errorf("Encode(%q, %q) = %s, want %s", tt.columnType, tt.input, got, tt.want) + } + if !json.Valid(got) { + t.Errorf("Encode(%q, %q) produced invalid JSON: %s", tt.columnType, tt.input, got) + } + }) + } +} + +func TestEncode_usageErrors(t *testing.T) { + tests := []struct { + name string + columnType string + settings string + input string + }{ + {name: "unknown column type", columnType: "mirror", input: "x"}, + {name: "status label not on column", columnType: "status", settings: statusSettingsObject, input: "Nearly Done"}, + {name: "status empty", columnType: "status", settings: statusSettingsObject, input: ""}, + {name: "date empty", columnType: "date", input: ""}, + {name: "date malformed", columnType: "date", input: "next tuesday"}, + {name: "date day-month order", columnType: "date", input: "10/05/2026"}, + {name: "numbers not numeric", columnType: "numbers", input: "many"}, + {name: "checkbox not boolean", columnType: "checkbox", input: "maybe"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Encode(tt.columnType, tt.settings, tt.input) + if err == nil { + t.Fatalf("Encode(%q, %q) = %s, want an error", tt.columnType, tt.input, got) + } + // The caller maps these straight to an exit code; a mislabelled encoder + // error would exit 5 (internal) on what is a caller mistake. + if code := codeOf(err); code != errs.CodeUsage { + t.Errorf("Encode(%q, %q) code = %s, want %s", tt.columnType, tt.input, code, errs.CodeUsage) + } + }) + } +} + +// TestEncode_statusErrorListsLabels checks the error is actionable: an agent that +// guessed a label needs to see the real ones, not just "invalid". +func TestEncode_statusErrorListsLabels(t *testing.T) { + _, err := Encode("status", statusSettingsObject, "Nearly Done") + if err == nil { + t.Fatal("want an error") + } + for _, label := range []string{"Not Started", "Working on it", "Done"} { + if !strings.Contains(err.Error(), label) { + t.Errorf("error %q does not name label %q", err.Error(), label) + } + } +} + +// TestEncoders_HaveDecoders guards the invariant stated in encoders' doc comment: a +// type mcli can write is a type mcli can read back, so a round-trip cannot lose data. +func TestEncoders_HaveDecoders(t *testing.T) { + for _, ct := range EncodableTypes() { + if _, ok := registry[ct]; !ok { + t.Errorf("column type %q has an encoder but no decoder", ct) + } + } +} + +// TestStatusLabels covers settings_str shapes, including the ones we cannot parse — +// there, no labels means "write it unvalidated" rather than "reject everything". +func TestStatusLabels(t *testing.T) { + tests := []struct { + name string + settings string + want []string + }{ + {name: "empty", settings: "", want: nil}, + {name: "object form in index order", settings: statusSettingsObject, want: []string{"Not Started", "Working on it", "Done"}}, + {name: "array form", settings: statusSettingsArray, want: []string{"Open", "Closed"}}, + {name: "unparseable", settings: "not json", want: nil}, + {name: "no labels key", settings: `{"labels_colors":{}}`, want: nil}, + {name: "blank labels dropped", settings: `{"labels":{"0":"","1":"Done"}}`, want: []string{"Done"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := statusLabels(tt.settings) + if len(got) != len(tt.want) { + t.Fatalf("statusLabels(%q) = %v, want %v", tt.settings, got, tt.want) + } + for i := range got { + if got[i] != tt.want[i] { + t.Fatalf("statusLabels(%q) = %v, want %v", tt.settings, got, tt.want) + } + } + }) + } +} + +// TestEncode_statusRoundTripsThroughDecode is the practical form of the +// encoder/decoder pairing: what we write is what a later read reports. +func TestEncode_statusRoundTripsThroughDecode(t *testing.T) { + raw, err := Encode("status", statusSettingsObject, "working on IT") + if err != nil { + t.Fatalf("Encode: %v", err) + } + decoded, err := Decode("status", statusSettingsObject, string(raw)) + if err != nil { + t.Fatalf("Decode: %v", err) + } + if decoded.Value != "Working on it" { + t.Errorf("round trip = %v, want %q", decoded.Value, "Working on it") + } +} diff --git a/internal/cli/item.go b/internal/cli/item.go index 0d43734..0a30b4b 100644 --- a/internal/cli/item.go +++ b/internal/cli/item.go @@ -130,110 +130,202 @@ type itemWriteOutput struct { ParentItem *itemWriteParent `json:"parent_item,omitempty"` } +// itemCreateToOutput converts a create_item response to the shared output shape. +func itemCreateToOutput(resp *gen.ItemCreateResponse) itemWriteOutput { + it := resp.Create_item + out := itemWriteOutput{ID: it.Id, Name: it.Name, State: string(it.State)} + if it.Board.Id != "" { + out.Board = &itemWriteBoard{ID: it.Board.Id, Name: it.Board.Name} + } + if it.Group.Id != "" { + out.Group = &itemWriteGroup{ID: it.Group.Id, Title: it.Group.Title} + } + return out +} + +// subitemCreateToOutput converts a create_subitem response to the shared output shape. +func subitemCreateToOutput(resp *gen.SubitemCreateResponse) itemWriteOutput { + it := resp.Create_subitem + out := itemWriteOutput{ID: it.Id, Name: it.Name, State: string(it.State)} + if it.Board.Id != "" { + out.Board = &itemWriteBoard{ID: it.Board.Id, Name: it.Board.Name} + } + if it.Parent_item.Id != "" { + out.ParentItem = &itemWriteParent{ID: it.Parent_item.Id, Name: it.Parent_item.Name} + } + return out +} + +// itemUpdateToOutput converts a change_multiple_column_values response to the shared +// output shape. +func itemUpdateToOutput(resp *gen.ItemUpdateResponse) itemWriteOutput { + it := resp.Change_multiple_column_values + out := itemWriteOutput{ID: it.Id, Name: it.Name, State: string(it.State)} + if it.Board.Id != "" { + out.Board = &itemWriteBoard{ID: it.Board.Id, Name: it.Board.Name} + } + if it.Group.Id != "" { + out.Group = &itemWriteGroup{ID: it.Group.Id, Title: it.Group.Title} + } + return out +} + +// checkBatchArgs validates the positional argument of a write command: either none, +// or the batch sentinel. Anything else is a caller mistake worth naming, since a bare +// item name as a positional arg is an easy thing to try. +func checkBatchArgs(args []string) error { + if len(args) == 0 || isBatchArg(args) { + return nil + } + return errs.Usage("unexpected argument %q: pass %q to read rows from stdin", args[0], batchStdinSentinel) +} + // --- item create --- +// itemCreateOpts holds the flags of 'mcli item create'. +type itemCreateOpts struct { + boardID string + parentID string + name string + groupID string + colFlags []string + dryRun bool + shorthands map[string]*string +} + func newItemCreateCmd() *cobra.Command { - var ( - boardID string - parentID string - name string - groupID string - colFlags []string - ) + opts := &itemCreateOpts{} cmd := &cobra.Command{ - Use: "create", + Use: "create [-]", Short: "Create an item (or subitem with --parent)", Long: `Create a monday.com item on a board, or a subitem under a parent item. Exactly one of --board or --parent must be provided. -Use --col = to set column values; the JSON must match monday's -column-value wire shape for the column type. Repeated --col flags are -order-preserving; if the same column id appears twice, the last value wins.`, - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, _ []string) error { - return runItemCreate(cmd, boardID, parentID, name, groupID, colFlags) + +Typed shorthands (--status, --date/--due, --number, --text, --checkbox) address the +board's single column of that type, so no column id is needed. They are validated +before anything is sent: a status label that is not on the column is rejected rather +than created. If a board has more than one column of the type, the shorthand is +ambiguous and errors — use --col for that write. + +Use --col = to set any column, including types with no shorthand; the JSON +must match monday's column-value wire shape for the column type. Repeated --col flags +are order-preserving; if the same column id appears twice, the last value wins. + +Pass - as the only argument to create many items from stdin, as a JSON array or one +JSON object per line. Each row takes "name", optional "group", optional "cols", and +the same shorthands as flags: + + echo '[{"name":"Ship v1","status":"Working on it","due":"2026-05-10"}]' \ + | mcli item create --board 123 - + +Rows are validated up front and sent sequentially. On partial failure the exit code is +2 and errors[].index names the rows to retry — do not re-send the whole batch, item +creation has no dedupe key.`, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runItemCreate(cmd, args, opts) }, } - cmd.Flags().StringVar(&boardID, "board", "", "board ID (required unless --parent is given)") - cmd.Flags().StringVar(&parentID, "parent", "", "parent item ID; creates a subitem when set") - cmd.Flags().StringVar(&name, "name", "", "item name (required)") - cmd.Flags().StringVar(&groupID, "group", "", "group ID (optional; ignored when --parent is given)") - cmd.Flags().StringArrayVar(&colFlags, "col", nil, "column value: = (repeatable)") + cmd.Flags().StringVar(&opts.boardID, "board", "", "board ID (required unless --parent is given)") + cmd.Flags().StringVar(&opts.parentID, "parent", "", "parent item ID; creates a subitem when set") + cmd.Flags().StringVar(&opts.name, "name", "", "item name (required unless reading rows from stdin)") + cmd.Flags().StringVar(&opts.groupID, "group", "", "group ID (optional; ignored when --parent is given)") + cmd.Flags().StringArrayVar(&opts.colFlags, "col", nil, "column value: = (repeatable)") + cmd.Flags().BoolVar(&opts.dryRun, "dry-run", false, "print what would be sent without creating anything") + opts.shorthands = addShorthandFlags(cmd) cmd.MarkFlagsMutuallyExclusive("board", "parent") - _ = cmd.MarkFlagRequired("name") return cmd } -func runItemCreate(cmd *cobra.Command, boardID, parentID, name, groupID string, colFlags []string) error { - if boardID == "" && parentID == "" { - return errs.Usage("one of --board or --parent is required") +func runItemCreate(cmd *cobra.Command, args []string, opts *itemCreateOpts) error { + if err := checkBatchArgs(args); err != nil { + return err } - if boardID != "" { - if _, err := strconv.ParseUint(boardID, 10, 64); err != nil { - return errs.Usage("board id must be a numeric string, got %q", boardID) + if opts.boardID == "" && opts.parentID == "" { + return errs.Usage("one of --board or --parent is required") + } + if opts.boardID != "" { + if _, err := strconv.ParseUint(opts.boardID, 10, 64); err != nil { + return errs.Usage("board id must be a numeric string, got %q", opts.boardID) } } - if parentID != "" { - if _, err := strconv.ParseUint(parentID, 10, 64); err != nil { - return errs.Usage("parent id must be a numeric string, got %q", parentID) + if opts.parentID != "" { + if _, err := strconv.ParseUint(opts.parentID, 10, 64); err != nil { + return errs.Usage("parent id must be a numeric string, got %q", opts.parentID) + } + } + + set := setShorthands(cmd, opts.shorthands) + + // Shorthands resolve against a board's columns. A subitem lives on its own + // hidden subitems board, which we would have to look up separately, so --parent + // keeps the raw path only. + if len(set) > 0 && opts.parentID != "" { + return errs.Usage("typed shorthands need --board; for a subitem use --col =") + } + + if isBatchArg(args) { + if opts.parentID != "" { + return errs.Usage("batch mode needs --board; subitems cannot be created from stdin") + } + if opts.name != "" || len(opts.colFlags) > 0 || len(set) > 0 { + return errs.Usage("batch mode takes every field from stdin: drop --name, --col, and the shorthand flags") } + return runItemCreateBatch(cmd, opts.boardID, opts.dryRun) } - cols, err := parseColFlags(colFlags) + if strings.TrimSpace(opts.name) == "" { + return errs.Usage("--name is required") + } + + cols, err := parseColFlags(opts.colFlags) if err != nil { return err } - colValuesStr, err := buildColumnValues(cols) + + // A dry run only needs the API to resolve shorthands. + var gql gqlclient.Client + if !opts.dryRun || len(set) > 0 { + gql, err = newItemClient() + if err != nil { + return err + } + } + + enc, err := applyShorthands(cmd.Context(), gql, opts.boardID, cols, set) if err != nil { return err } - gql, err := newItemClient() + colValuesStr, err := buildColumnValues(cols) if err != nil { return err } - var out itemWriteOutput + if opts.dryRun { + row := batchRow{Name: opts.name, Group: opts.groupID} + return writeBatchDryRun(cmd, "created", []batchRow{row}, []string{colValuesStr}) + } - if parentID != "" { - resp, apiErr := gen.SubitemCreate(cmd.Context(), gql, parentID, name, colValuesStr) + var out itemWriteOutput + if opts.parentID != "" { + resp, apiErr := gen.SubitemCreate(cmd.Context(), gql, opts.parentID, opts.name, colValuesStr) if apiErr != nil { return apiErr } - it := resp.Create_subitem - out = itemWriteOutput{ - ID: it.Id, - Name: it.Name, - State: string(it.State), - } - if it.Board.Id != "" { - out.Board = &itemWriteBoard{ID: it.Board.Id, Name: it.Board.Name} - } - if it.Parent_item.Id != "" { - out.ParentItem = &itemWriteParent{ID: it.Parent_item.Id, Name: it.Parent_item.Name} - } + out = subitemCreateToOutput(resp) } else { - resp, apiErr := gen.ItemCreate(cmd.Context(), gql, boardID, name, groupID, colValuesStr) + resp, apiErr := gen.ItemCreate(cmd.Context(), gql, opts.boardID, opts.name, opts.groupID, colValuesStr) if apiErr != nil { return apiErr } - it := resp.Create_item - out = itemWriteOutput{ - ID: it.Id, - Name: it.Name, - State: string(it.State), - } - if it.Board.Id != "" { - out.Board = &itemWriteBoard{ID: it.Board.Id, Name: it.Board.Name} - } - if it.Group.Id != "" { - out.Group = &itemWriteGroup{ID: it.Group.Id, Title: it.Group.Title} - } + out = itemCreateToOutput(resp) } mode, modeErr := resolveOutputMode(os.Stdout, globals, configOutputMode()) @@ -251,65 +343,119 @@ func runItemCreate(cmd *cobra.Command, boardID, parentID, name, groupID string, } kind := "item" - if parentID != "" { + if opts.parentID != "" { kind = "subitem" } - _, err = fmt.Fprintf(cmd.OutOrStdout(), "Created %s %s: %s\n", kind, out.ID, out.Name) - return err + o := cmd.OutOrStdout() + if _, err = fmt.Fprintf(o, "Created %s %s: %s\n", kind, out.ID, out.Name); err != nil { + return err + } + writeShorthandTrace(o, enc) + return nil +} + +// writeShorthandTrace reports which column each shorthand landed on. Pretty output is +// for humans, and "--status went to status_1 (Stage)" is the one thing a human cannot +// see from the command they typed. +func writeShorthandTrace(o io.Writer, enc []encodedShorthand) { + for _, e := range enc { + _, _ = fmt.Fprintf(o, " --%s → %s (%s) = %s\n", e.flag, e.colID, e.title, string(e.value)) + } } // --- item update --- +// itemUpdateOpts holds the flags of 'mcli item update'. +type itemUpdateOpts struct { + boardID string + name string + colFlags []string + dryRun bool + shorthands map[string]*string +} + func newItemUpdateCmd() *cobra.Command { - var ( - boardID string - name string - colFlags []string - ) + opts := &itemUpdateOpts{} cmd := &cobra.Command{ - Use: "update ", + Use: "update | -", Short: "Update column values on an item", Long: `Update one or more column values on a monday.com item. -Provide --name to rename the item. Use --col = to update columns; -the JSON must match monday's column-value wire shape. At least one of --name -or --col must be given.`, +Provide --name to rename the item. Typed shorthands (--status, --date/--due, +--number, --text, --checkbox) address the board's single column of that type and are +validated before anything is sent. Use --col = for any other column; the +JSON must match monday's column-value wire shape. At least one of --name, --col, or a +shorthand must be given. + +Pass - instead of an item id to update many items from stdin, as a JSON array or one +JSON object per line. Each row needs "id" and takes optional "name", "cols", and the +same shorthands as flags: + + echo '{"id":"456","status":"Done"}' | mcli item update --board 123 - + +Rows are validated up front and sent sequentially. On partial failure the exit code is +2 and errors[].index names the rows to retry.`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return runItemUpdate(cmd, args[0], boardID, name, colFlags) + return runItemUpdate(cmd, args[0], opts) }, } - cmd.Flags().StringVar(&boardID, "board", "", "board ID (required)") - cmd.Flags().StringVar(&name, "name", "", "new item name (optional)") - cmd.Flags().StringArrayVar(&colFlags, "col", nil, "column value: = (repeatable)") + cmd.Flags().StringVar(&opts.boardID, "board", "", "board ID (required)") + cmd.Flags().StringVar(&opts.name, "name", "", "new item name (optional)") + cmd.Flags().StringArrayVar(&opts.colFlags, "col", nil, "column value: = (repeatable)") + cmd.Flags().BoolVar(&opts.dryRun, "dry-run", false, "print what would be sent without updating anything") + opts.shorthands = addShorthandFlags(cmd) + _ = cmd.MarkFlagRequired("board") return cmd } -func runItemUpdate(cmd *cobra.Command, itemID, boardID, name string, colFlags []string) error { +func runItemUpdate(cmd *cobra.Command, itemID string, opts *itemUpdateOpts) error { + if _, err := strconv.ParseUint(opts.boardID, 10, 64); err != nil { + return errs.Usage("board id must be a numeric string, got %q", opts.boardID) + } + + set := setShorthands(cmd, opts.shorthands) + + if itemID == batchStdinSentinel { + if opts.name != "" || len(opts.colFlags) > 0 || len(set) > 0 { + return errs.Usage("batch mode takes every field from stdin: drop --name, --col, and the shorthand flags") + } + return runItemUpdateBatch(cmd, opts.boardID, opts.dryRun) + } + if _, err := strconv.ParseUint(itemID, 10, 64); err != nil { return errs.Usage("item id must be a numeric string, got %q", itemID) } - if _, err := strconv.ParseUint(boardID, 10, 64); err != nil { - return errs.Usage("board id must be a numeric string, got %q", boardID) + if opts.name == "" && len(opts.colFlags) == 0 && len(set) == 0 { + return errs.Usage("nothing to update: provide --name, at least one --col, or a shorthand") + } + + cols, err := parseColFlags(opts.colFlags) + if err != nil { + return err } - if name == "" && len(colFlags) == 0 { - return errs.Usage("nothing to update: provide --name and/or at least one --col") + var gql gqlclient.Client + if !opts.dryRun || len(set) > 0 { + gql, err = newItemClient() + if err != nil { + return err + } } - cols, err := parseColFlags(colFlags) + enc, err := applyShorthands(cmd.Context(), gql, opts.boardID, cols, set) if err != nil { return err } // If --name is provided, inject the name column. Monday expects a bare // JSON string for the "name" column in change_multiple_column_values. - if name != "" { - nameVal, _ := json.Marshal(name) + if opts.name != "" { + nameVal, _ := json.Marshal(opts.name) cols["name"] = json.RawMessage(nameVal) } @@ -318,28 +464,16 @@ func runItemUpdate(cmd *cobra.Command, itemID, boardID, name string, colFlags [] return err } - gql, err := newItemClient() - if err != nil { - return err + if opts.dryRun { + row := batchRow{ID: itemID, Name: opts.name} + return writeBatchDryRun(cmd, "updated", []batchRow{row}, []string{colValuesStr}) } - resp, apiErr := gen.ItemUpdate(cmd.Context(), gql, boardID, itemID, colValuesStr) + resp, apiErr := gen.ItemUpdate(cmd.Context(), gql, opts.boardID, itemID, colValuesStr) if apiErr != nil { return apiErr } - - it := resp.Change_multiple_column_values - out := itemWriteOutput{ - ID: it.Id, - Name: it.Name, - State: string(it.State), - } - if it.Board.Id != "" { - out.Board = &itemWriteBoard{ID: it.Board.Id, Name: it.Board.Name} - } - if it.Group.Id != "" { - out.Group = &itemWriteGroup{ID: it.Group.Id, Title: it.Group.Title} - } + out := itemUpdateToOutput(resp) mode, modeErr := resolveOutputMode(os.Stdout, globals, configOutputMode()) if modeErr != nil { @@ -356,14 +490,17 @@ func runItemUpdate(cmd *cobra.Command, itemID, boardID, name string, colFlags [] } o := cmd.OutOrStdout() - _, err = fmt.Fprintf(o, "Updated item %s\n", out.ID) - if name != "" { - _, _ = fmt.Fprintf(o, " name → %s\n", name) + if _, err = fmt.Fprintf(o, "Updated item %s\n", out.ID); err != nil { + return err + } + if opts.name != "" { + _, _ = fmt.Fprintf(o, " name → %s\n", opts.name) } - for _, f := range colFlags { + for _, f := range opts.colFlags { _, _ = fmt.Fprintf(o, " col → %s\n", f) } - return err + writeShorthandTrace(o, enc) + return nil } // --- item post-update --- diff --git a/internal/cli/item_batch.go b/internal/cli/item_batch.go new file mode 100644 index 0000000..a1c8c6b --- /dev/null +++ b/internal/cli/item_batch.go @@ -0,0 +1,565 @@ +package cli + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "maps" + "os" + "strconv" + "strings" + + gqlclient "github.com/Khan/genqlient/graphql" + "github.com/spf13/cobra" + + "github.com/mondaycom/mcli/internal/api/gen" + apigraphql "github.com/mondaycom/mcli/internal/api/graphql" + "github.com/mondaycom/mcli/internal/errs" +) + +const ( + // batchStdinSentinel is the positional arg that switches create/update into + // batch mode, reading rows from stdin. ADR-002 specifies this contract. + batchStdinSentinel = "-" + // batchMaxInputBytes bounds stdin so a runaway pipe cannot exhaust memory. + batchMaxInputBytes = 8 << 20 + // batchMaxRows caps one invocation. create_item has no dedupe key, so an + // accidental huge batch is expensive to undo; splitting is the caller's call. + batchMaxRows = 500 +) + +// jsonScalar is a JSON string, number, or boolean captured as its text form, so a +// batch row may write "number": 42 or "number": "42" and mean the same thing. +type jsonScalar string + +// UnmarshalJSON accepts a string, number, or boolean. +func (s *jsonScalar) UnmarshalJSON(b []byte) error { + // null is rejected rather than read as "clear the column": encoding/json treats + // unmarshalling null as a no-op, so it would silently mean "absent" instead. Use + // "" to clear a column. + if string(bytes.TrimSpace(b)) == "null" { + return fmt.Errorf(`null is not a value; use "" to clear a column`) + } + + var str string + if err := json.Unmarshal(b, &str); err == nil { + *s = jsonScalar(str) + return nil + } + var num json.Number + if err := json.Unmarshal(b, &num); err == nil { + *s = jsonScalar(num.String()) + return nil + } + var boolean bool + if err := json.Unmarshal(b, &boolean); err == nil { + *s = jsonScalar(strconv.FormatBool(boolean)) + return nil + } + return fmt.Errorf("expected a string, number, or boolean, got %s", string(b)) +} + +// MarshalJSON keeps --dry-run echoes round-trippable. +func (s jsonScalar) MarshalJSON() ([]byte, error) { + return json.Marshal(string(s)) +} + +// batchRow is one row of a batch payload. Create rows carry name/group, update rows +// carry id, and both may carry raw cols plus the same typed shorthands as the flags. +// +// A nil shorthand pointer means "absent"; an empty one means "write empty", which is +// how a column gets cleared. +type batchRow struct { + ID string `json:"id,omitempty"` + Name string `json:"name,omitempty"` + Group string `json:"group,omitempty"` + Cols map[string]json.RawMessage `json:"cols,omitempty"` + + Text *jsonScalar `json:"text,omitempty"` + Status *jsonScalar `json:"status,omitempty"` + Date *jsonScalar `json:"date,omitempty"` + Due *jsonScalar `json:"due,omitempty"` + Number *jsonScalar `json:"number,omitempty"` + Checkbox *jsonScalar `json:"checkbox,omitempty"` +} + +// shorthands returns the shorthands present on the row, in spec order, so the stdin +// path and the flag path share one encoder and one resolution rule. +func (r batchRow) shorthands() []shorthandValue { + byFlag := map[string]*jsonScalar{ + "text": r.Text, + "status": r.Status, + "date": r.Date, + "due": r.Due, + "number": r.Number, + "checkbox": r.Checkbox, + } + var out []shorthandValue + for _, sp := range shorthandSpecs { + if v := byFlag[sp.flag]; v != nil { + out = append(out, shorthandValue{spec: sp, value: string(*v)}) + } + } + return out +} + +// isBatchArg reports whether args select batch mode. +func isBatchArg(args []string) bool { + return len(args) == 1 && args[0] == batchStdinSentinel +} + +// parseBatchRows reads a JSON array or an NDJSON stream of rows. +// +// Unknown fields are rejected: a mistyped key like "columns" instead of "cols" would +// otherwise silently drop every column value in the row and report success. +func parseBatchRows(r io.Reader) ([]batchRow, error) { + data, err := io.ReadAll(io.LimitReader(r, batchMaxInputBytes+1)) + if err != nil { + return nil, errs.Usage("read batch rows from stdin: %v", err) + } + if len(data) > batchMaxInputBytes { + return nil, errs.Usage("batch input exceeds %d bytes; split it into smaller batches", batchMaxInputBytes) + } + + trimmed := bytes.TrimSpace(data) + if len(trimmed) == 0 { + return nil, errs.Usage("no batch rows on stdin") + } + + dec := json.NewDecoder(bytes.NewReader(trimmed)) + dec.DisallowUnknownFields() + + var rows []batchRow + if trimmed[0] == '[' { + if err := dec.Decode(&rows); err != nil { + return nil, errs.Usage("parse batch rows: %v", err) + } + if dec.More() { + return nil, errs.Usage("parse batch rows: unexpected content after the JSON array") + } + } else { + for { + var row batchRow + if err := dec.Decode(&row); err != nil { + if errors.Is(err, io.EOF) { + break + } + return nil, errs.Usage("parse batch row %d: %v", len(rows), err) + } + rows = append(rows, row) + } + } + + if len(rows) == 0 { + return nil, errs.Usage("no batch rows on stdin") + } + if len(rows) > batchMaxRows { + return nil, errs.Usage("batch has %d rows, max is %d; split it into smaller batches", len(rows), batchMaxRows) + } + return rows, nil +} + +// validateBatchRow checks what is common to create and update rows. +func validateBatchRow(index int, row batchRow) error { + if row.Date != nil && row.Due != nil { + return errs.Usage("row %d: set either \"date\" or \"due\", not both", index) + } + for id, raw := range row.Cols { + if id == "" { + return errs.Usage("row %d: cols has an empty column id", index) + } + if !json.Valid(raw) { + return errs.Usage("row %d: cols.%s is not valid JSON", index, id) + } + } + return nil +} + +// validateCreateRows rejects the whole payload before any request fires: a batch that +// dies halfway through leaves items behind that a retry would duplicate. +func validateCreateRows(rows []batchRow) error { + for i, row := range rows { + if err := validateBatchRow(i, row); err != nil { + return err + } + if strings.TrimSpace(row.Name) == "" { + return errs.Usage("row %d: \"name\" is required to create an item", i) + } + if row.ID != "" { + return errs.Usage("row %d: \"id\" is not valid when creating items", i) + } + } + return nil +} + +// validateUpdateRows rejects the whole payload before any request fires. +func validateUpdateRows(rows []batchRow) error { + for i, row := range rows { + if err := validateBatchRow(i, row); err != nil { + return err + } + if row.ID == "" { + return errs.Usage("row %d: \"id\" is required to update an item", i) + } + if _, err := strconv.ParseUint(row.ID, 10, 64); err != nil { + return errs.Usage("row %d: id must be a numeric string, got %q", i, row.ID) + } + if row.Name == "" && len(row.Cols) == 0 && len(row.shorthands()) == 0 { + return errs.Usage("row %d: nothing to update: provide \"name\", \"cols\", or a shorthand", i) + } + } + return nil +} + +// batchNeedsColumns reports whether any row uses a shorthand, i.e. whether the +// board's columns have to be fetched at all. +func batchNeedsColumns(rows []batchRow) bool { + for _, row := range rows { + if len(row.shorthands()) > 0 { + return true + } + } + return false +} + +// buildBatchPayloads turns every row into the exact column_values string that will be +// sent for it, resolving the board's columns at most once for the whole batch. +// +// All rows are built up front so a bad status label fails the batch as a usage error +// instead of surfacing after the first N items are already created. +func buildBatchPayloads( + ctx context.Context, gql gqlclient.Client, boardID string, rows []batchRow, +) ([]string, error) { + var idx *boardColumnIndex + if batchNeedsColumns(rows) { + var err error + idx, err = fetchBoardColumnIndex(ctx, gql, boardID) + if err != nil { + return nil, err + } + } + + payloads := make([]string, len(rows)) + for i, row := range rows { + // Copied rather than used in place, so encoding a shorthand never mutates the + // caller's parsed row (--dry-run echoes it back). + cols := make(map[string]json.RawMessage, len(row.Cols)) + maps.Copy(cols, row.Cols) + + if short := row.shorthands(); len(short) > 0 { + enc, err := encodeShorthands(idx, short) + if err != nil { + return nil, errs.Usage("row %d: %s", i, errMessage(err)) + } + if err := mergeShorthands(cols, enc); err != nil { + return nil, errs.Usage("row %d: %s", i, errMessage(err)) + } + } + + // change_multiple_column_values takes the item name as a "name" column. + if row.Name != "" && row.ID != "" { + nameVal, _ := json.Marshal(row.Name) + cols["name"] = json.RawMessage(nameVal) + } + + payload, err := buildColumnValues(cols) + if err != nil { + return nil, errs.Usage("row %d: %v", i, err) + } + payloads[i] = payload + } + return payloads, nil +} + +// errMessage returns an *errs.Error's bare message, so nesting a row index in front +// of it does not produce a doubled "[USAGE] … [USAGE] …" string. +func errMessage(err error) string { + if e, ok := errors.AsType[*errs.Error](err); ok { + return e.Message + } + return err.Error() +} + +// batchError reports one failed row. +// +// Index is the row's position in the input, which is what makes a partial failure +// recoverable: create_item has no dedupe key, so re-running the whole payload +// duplicates every row that succeeded. Retry the listed indexes, not the batch. +type batchError struct { + Index int `json:"index"` + ID string `json:"id,omitempty"` + Code string `json:"code"` + Message string `json:"message"` +} + +// batchWriteOutput is the JSON shape for a batch create or update. It is one +// top-level object, never a stream (ADR-002), and every key is always present so a +// caller can read counts without checking for absence. +type batchWriteOutput struct { + // Written is the number of rows monday accepted. + Written int `json:"written"` + Failed int `json:"failed"` + // Verb is "created" or "updated", so a caller need not infer it from the command. + Verb string `json:"verb"` + Items []itemWriteOutput `json:"items"` + Errors []batchError `json:"errors"` +} + +// batchDryRunRow echoes one row exactly as it would be sent. +type batchDryRunRow struct { + Index int `json:"index"` + ID string `json:"id,omitempty"` + Name string `json:"name,omitempty"` + Group string `json:"group,omitempty"` + ColumnValues string `json:"column_values,omitempty"` +} + +// batchDryRunOutput is the JSON shape for a batch --dry-run. +type batchDryRunOutput struct { + DryRun bool `json:"dry_run"` + Verb string `json:"verb"` + Rows int `json:"rows"` + Items []batchDryRunRow `json:"items"` +} + +// classifyBatchErr maps a per-row error to a stable code. Errors from the production +// client are already *errs.Error; anything else is normalised so a raw transport +// failure still reports a real code rather than a bare INTERNAL. +func classifyBatchErr(err error) errs.Code { + if e, ok := errors.AsType[*errs.Error](err); ok { + return e.Code + } + if e, ok := errors.AsType[*errs.Error](apigraphql.Normalize(err)); ok { + return e.Code + } + return errs.CodeInternal +} + +// batchRowFn performs the write for one row, returning the item monday reported. +type batchRowFn func(ctx context.Context, index int, row batchRow, columnValues string) (itemWriteOutput, error) + +// runBatch executes rows sequentially and collects per-row outcomes. +// +// Sequential is deliberate: monday's limit is complexity-per-minute, so parallelism +// does not raise throughput, it only reaches the ceiling sooner and makes pacing +// impossible to reason about. +func runBatch(ctx context.Context, verb string, rows []batchRow, payloads []string, write batchRowFn) batchWriteOutput { + out := batchWriteOutput{ + Verb: verb, + Items: make([]itemWriteOutput, 0, len(rows)), + Errors: []batchError{}, + } + + for i, row := range rows { + if ctxErr := ctx.Err(); ctxErr != nil { + out.Errors = append(out.Errors, batchError{ + Index: i, + ID: row.ID, + Code: string(errs.CodeInterrupted), + Message: fmt.Sprintf("cancelled before row %d of %d", i, len(rows)), + }) + out.Failed++ + break + } + + item, err := write(ctx, i, row, payloads[i]) + if err != nil { + out.Errors = append(out.Errors, batchError{ + Index: i, + ID: row.ID, + Code: string(classifyBatchErr(err)), + Message: errMessage(err), + }) + out.Failed++ + continue + } + out.Items = append(out.Items, item) + out.Written++ + } + + return out +} + +// writeBatchOutput renders the batch result and returns the error that sets the exit +// code: any failed row is exit 2, so a caller can tell "all fine" from "mostly fine" +// without diffing counts. +func writeBatchOutput(cmd *cobra.Command, out batchWriteOutput) error { + mode, modeErr := resolveOutputMode(os.Stdout, globals, configOutputMode()) + if modeErr != nil { + return modeErr + } + + o := cmd.OutOrStdout() + if mode == ModeJSON { + data, mErr := json.Marshal(out) + if mErr != nil { + return errs.Internal("marshal output: %v", mErr) + } + if _, err := fmt.Fprintln(o, string(data)); err != nil { + return err + } + } else { + if _, err := fmt.Fprintf(o, "%s %s, %d failed\n", out.Verb, pluralItems(out.Written), out.Failed); err != nil { + return err + } + for _, e := range out.Errors { + _, _ = fmt.Fprintf(o, " row %d: [%s] %s\n", e.Index, e.Code, e.Message) + } + } + + if out.Failed > 0 { + return errs.API("%d of %d rows failed; see errors[].index to retry only those", + out.Failed, out.Written+out.Failed) + } + return nil +} + +// writeBatchDryRun renders what a batch would send, without sending it. +func writeBatchDryRun(cmd *cobra.Command, verb string, rows []batchRow, payloads []string) error { + out := batchDryRunOutput{DryRun: true, Verb: verb, Rows: len(rows), Items: make([]batchDryRunRow, 0, len(rows))} + for i, row := range rows { + out.Items = append(out.Items, batchDryRunRow{ + Index: i, + ID: row.ID, + Name: row.Name, + Group: row.Group, + ColumnValues: payloads[i], + }) + } + + mode, modeErr := resolveOutputMode(os.Stdout, globals, configOutputMode()) + if modeErr != nil { + return modeErr + } + + o := cmd.OutOrStdout() + if mode == ModeJSON { + data, mErr := json.Marshal(out) + if mErr != nil { + return errs.Internal("marshal output: %v", mErr) + } + _, err := fmt.Fprintln(o, string(data)) + return err + } + + if _, err := fmt.Fprintf(o, "dry run: would %s %s\n", verbInfinitive(verb), pluralItems(len(rows))); err != nil { + return err + } + for _, it := range out.Items { + label := it.Name + if it.ID != "" { + label = it.ID + } + line := fmt.Sprintf(" row %d: %s", it.Index, label) + if it.ColumnValues != "" { + line += " " + it.ColumnValues + } + _, _ = fmt.Fprintln(o, line) + } + return nil +} + +// verbInfinitive turns the output verb into the form a sentence needs. +func verbInfinitive(verb string) string { + switch verb { + case "created": + return "create" + case "updated": + return "update" + default: + return verb + } +} + +// pluralItems renders an item count without the "1 items" tell. +func pluralItems(n int) string { + if n == 1 { + return "1 item" + } + return fmt.Sprintf("%d items", n) +} + +// --- batch entry points --- + +// runItemCreateBatch implements 'mcli item create --board -'. +func runItemCreateBatch(cmd *cobra.Command, boardID string, dryRun bool) error { + rows, err := parseBatchRows(cmd.InOrStdin()) + if err != nil { + return err + } + if err := validateCreateRows(rows); err != nil { + return err + } + + // A dry run that uses no shorthand needs no API access at all, so it stays + // usable as a pure parse check without a token. + var gql gqlclient.Client + if !dryRun || batchNeedsColumns(rows) { + gql, err = newItemClient() + if err != nil { + return err + } + } + + payloads, err := buildBatchPayloads(cmd.Context(), gql, boardID, rows) + if err != nil { + return err + } + + if dryRun { + return writeBatchDryRun(cmd, "created", rows, payloads) + } + + out := runBatch(cmd.Context(), "created", rows, payloads, + func(ctx context.Context, _ int, row batchRow, columnValues string) (itemWriteOutput, error) { + resp, apiErr := gen.ItemCreate(ctx, gql, boardID, row.Name, row.Group, columnValues) + if apiErr != nil { + return itemWriteOutput{}, apiErr + } + return itemCreateToOutput(resp), nil + }) + + return writeBatchOutput(cmd, out) +} + +// runItemUpdateBatch implements 'mcli item update --board -'. +func runItemUpdateBatch(cmd *cobra.Command, boardID string, dryRun bool) error { + rows, err := parseBatchRows(cmd.InOrStdin()) + if err != nil { + return err + } + if err := validateUpdateRows(rows); err != nil { + return err + } + + var gql gqlclient.Client + if !dryRun || batchNeedsColumns(rows) { + gql, err = newItemClient() + if err != nil { + return err + } + } + + payloads, err := buildBatchPayloads(cmd.Context(), gql, boardID, rows) + if err != nil { + return err + } + + if dryRun { + return writeBatchDryRun(cmd, "updated", rows, payloads) + } + + out := runBatch(cmd.Context(), "updated", rows, payloads, + func(ctx context.Context, _ int, row batchRow, columnValues string) (itemWriteOutput, error) { + resp, apiErr := gen.ItemUpdate(ctx, gql, boardID, row.ID, columnValues) + if apiErr != nil { + return itemWriteOutput{}, apiErr + } + return itemUpdateToOutput(resp), nil + }) + + return writeBatchOutput(cmd, out) +} diff --git a/internal/cli/item_batch_test.go b/internal/cli/item_batch_test.go new file mode 100644 index 0000000..fd718cf --- /dev/null +++ b/internal/cli/item_batch_test.go @@ -0,0 +1,914 @@ +package cli + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "github.com/mondaycom/mcli/internal/errs" +) + +// gqlCall records one request the CLI made. +type gqlCall struct { + op string + vars map[string]any +} + +// gqlRecorder is a fake monday endpoint that records every call and lets a test +// return a full response body (data or errors) per request. +type gqlRecorder struct { + mu sync.Mutex + calls []gqlCall +} + +// newGQLRecorder starts a server whose handler is fn(op, vars, nthCallOfThatOp) and +// returns the raw GraphQL response body to send. +func newGQLRecorder(t *testing.T, fn func(op string, vars map[string]any, n int) string) (*gqlRecorder, *httptest.Server) { + t.Helper() + rec := &gqlRecorder{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + raw, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, "read body", http.StatusBadRequest) + return + } + var body map[string]any + if err := json.Unmarshal(raw, &body); err != nil { + http.Error(w, "parse body", http.StatusBadRequest) + return + } + op, _ := body["operationName"].(string) + vars, _ := body["variables"].(map[string]any) + + rec.mu.Lock() + n := 0 + for _, c := range rec.calls { + if c.op == op { + n++ + } + } + rec.calls = append(rec.calls, gqlCall{op: op, vars: vars}) + rec.mu.Unlock() + + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprint(w, fn(op, vars, n)) + })) + t.Cleanup(srv.Close) + installItemFactory(t, srv.URL) + return rec, srv +} + +// ops returns the recorded operation names in order. +func (r *gqlRecorder) ops() []string { + r.mu.Lock() + defer r.mu.Unlock() + out := make([]string, 0, len(r.calls)) + for _, c := range r.calls { + out = append(out, c.op) + } + return out +} + +// countOp returns how many times op was called. +func (r *gqlRecorder) countOp(op string) int { + n := 0 + for _, got := range r.ops() { + if got == op { + n++ + } + } + return n +} + +// varsFor returns the variables of the nth (0-based) call to op. +func (r *gqlRecorder) varsFor(op string, n int) map[string]any { + r.mu.Lock() + defer r.mu.Unlock() + seen := 0 + for _, c := range r.calls { + if c.op != op { + continue + } + if seen == n { + return c.vars + } + seen++ + } + return nil +} + +// boardColumnsBody is a BoardColumnList response for a board with one column of each +// shorthand-addressable type, plus an archived status column that must be ignored (it +// would otherwise make --status ambiguous even though it cannot be written). +func boardColumnsBody() string { + cols := []map[string]any{ + {"id": "name", "title": "Name", "type": "name", "settings_str": "{}", "width": 200, "archived": false}, + {"id": "status_1", "title": "Stage", "type": "status", "settings_str": testStatusSettings, "width": 150, "archived": false}, + {"id": "status_old", "title": "Old Stage", "type": "status", "settings_str": testStatusSettings, "width": 150, "archived": true}, + {"id": "date_4", "title": "Due", "type": "date", "settings_str": "{}", "width": 150, "archived": false}, + {"id": "numbers_7", "title": "Estimate", "type": "numbers", "settings_str": "{}", "width": 150, "archived": false}, + {"id": "text_9", "title": "Notes", "type": "text", "settings_str": "{}", "width": 150, "archived": false}, + {"id": "checkbox_2", "title": "Blocked", "type": "checkbox", "settings_str": "{}", "width": 150, "archived": false}, + } + boards := []map[string]any{{"id": "9832181507", "columns": cols}} + return fmt.Sprintf(`{"data":%s}`, mustMarshal(map[string]any{"boards": boards})) +} + +// boardColumnsBodyTextOnly is a board with no column of most shorthand types, for the +// "this board has none" path. +func boardColumnsBodyTextOnly() string { + cols := []map[string]any{ + {"id": "name", "title": "Name", "type": "name", "settings_str": "{}", "width": 200, "archived": false}, + {"id": "text_9", "title": "Notes", "type": "text", "settings_str": "{}", "width": 150, "archived": false}, + } + boards := []map[string]any{{"id": "9832181507", "columns": cols}} + return fmt.Sprintf(`{"data":%s}`, mustMarshal(map[string]any{"boards": boards})) +} + +// itemCreateBody is a successful create_item response. +func itemCreateBody(id, name string) string { + return fmt.Sprintf(`{"data":{"create_item":{"id":%q,"name":%q,"state":"active",`+ + `"board":{"id":"9832181507","name":"Dev Board"},"group":{"id":"topics","title":"Sprint 1"}}}}`, id, name) +} + +// itemUpdateBody is a successful change_multiple_column_values response. +func itemUpdateBody(id, name string) string { + return fmt.Sprintf(`{"data":{"change_multiple_column_values":{"id":%q,"name":%q,"state":"active",`+ + `"board":{"id":"9832181507","name":"Dev Board"},"group":{"id":"topics","title":"Sprint 1"}}}}`, id, name) +} + +// gqlErrorBody is a GraphQL error response. +func gqlErrorBody(message, code string) string { + return fmt.Sprintf(`{"errors":[{"message":%q,"extensions":{"code":%q}}]}`, message, code) +} + +// execItemCreateStdin runs 'item create' with stdin wired to in. +func execItemCreateStdin(t *testing.T, in string, args ...string) (string, error) { + t.Helper() + globals = GlobalFlags{JSON: true} + defer func() { globals = GlobalFlags{} }() + + var buf bytes.Buffer + cmd := newItemCmd() + // Mirror the production root command: usage text must not land on stdout, which + // batch mode uses for its JSON result even when it exits non-zero. + cmd.SilenceUsage = true + cmd.SilenceErrors = true + cmd.SetOut(&buf) + cmd.SetErr(io.Discard) + cmd.SetIn(strings.NewReader(in)) + cmd.SetArgs(append([]string{"create"}, args...)) + err := cmd.Execute() + return buf.String(), err +} + +// execItemUpdateStdin runs 'item update' with stdin wired to in. +func execItemUpdateStdin(t *testing.T, in string, args ...string) (string, error) { + t.Helper() + globals = GlobalFlags{JSON: true} + defer func() { globals = GlobalFlags{} }() + + var buf bytes.Buffer + cmd := newItemCmd() + // Mirror the production root command: usage text must not land on stdout, which + // batch mode uses for its JSON result even when it exits non-zero. + cmd.SilenceUsage = true + cmd.SilenceErrors = true + cmd.SetOut(&buf) + cmd.SetErr(io.Discard) + cmd.SetIn(strings.NewReader(in)) + cmd.SetArgs(append([]string{"update"}, args...)) + err := cmd.Execute() + return buf.String(), err +} + +// decodeBatch parses batch output. +func decodeBatch(t *testing.T, out string) batchWriteOutput { + t.Helper() + var got batchWriteOutput + if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &got); err != nil { + t.Fatalf("parse output: %v\nraw: %s", err, out) + } + return got +} + +// ---- jsonScalar ---- + +// TestJSONScalar accepts the three JSON scalar forms, so a row may write +// "number": 42 without quoting and "checkbox": true without stringifying. +func TestJSONScalar(t *testing.T) { + tests := []struct { + in string + want string + }{ + {in: `"Done"`, want: "Done"}, + {in: `42`, want: "42"}, + {in: `42.5`, want: "42.5"}, + {in: `true`, want: "true"}, + {in: `false`, want: "false"}, + {in: `""`, want: ""}, + } + for _, tt := range tests { + var got jsonScalar + if err := json.Unmarshal([]byte(tt.in), &got); err != nil { + t.Errorf("Unmarshal(%s): %v", tt.in, err) + continue + } + if string(got) != tt.want { + t.Errorf("Unmarshal(%s) = %q, want %q", tt.in, got, tt.want) + } + } + + for _, bad := range []string{`null`, `[1]`, `{"a":1}`} { + var got jsonScalar + if err := json.Unmarshal([]byte(bad), &got); err == nil { + t.Errorf("Unmarshal(%s) = %q, want an error", bad, got) + } + } +} + +// ---- parsing ---- + +func TestParseBatchRows_arrayAndNDJSON(t *testing.T) { + array := `[{"name":"a"},{"name":"b"}]` + ndjson := "{\"name\":\"a\"}\n{\"name\":\"b\"}\n" + + for _, in := range []string{array, ndjson} { + rows, err := parseBatchRows(strings.NewReader(in)) + if err != nil { + t.Fatalf("parseBatchRows(%q): %v", in, err) + } + if len(rows) != 2 || rows[0].Name != "a" || rows[1].Name != "b" { + t.Errorf("parseBatchRows(%q) = %+v", in, rows) + } + } +} + +// TestParseBatchRows_rejectsUnknownField is why DisallowUnknownFields is set: a +// mistyped key would otherwise drop the caller's data and report success. +func TestParseBatchRows_rejectsUnknownField(t *testing.T) { + _, err := parseBatchRows(strings.NewReader(`[{"name":"a","columns":{"x":1}}]`)) + if err == nil { + t.Fatal("want an error for an unknown row field") + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } +} + +func TestParseBatchRows_errors(t *testing.T) { + tests := []struct { + name string + in string + }{ + {name: "empty", in: ""}, + {name: "whitespace only", in: " \n\t\n"}, + {name: "empty array", in: "[]"}, + {name: "truncated array", in: `[{"name":"a"}`}, + {name: "trailing content after array", in: `[{"name":"a"}] {"name":"b"}`}, + {name: "not an object", in: `"a string"`}, + {name: "bad ndjson second row", in: "{\"name\":\"a\"}\nnope\n"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rows, err := parseBatchRows(strings.NewReader(tt.in)) + if err == nil { + t.Fatalf("parseBatchRows(%q) = %+v, want an error", tt.in, rows) + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } + }) + } +} + +func TestParseBatchRows_maxRows(t *testing.T) { + rows := make([]string, batchMaxRows) + for i := range rows { + rows[i] = fmt.Sprintf(`{"name":"item %d"}`, i) + } + atMax := "[" + strings.Join(rows, ",") + "]" + + got, err := parseBatchRows(strings.NewReader(atMax)) + if err != nil { + t.Fatalf("parseBatchRows at max (%d rows): %v", batchMaxRows, err) + } + if len(got) != batchMaxRows { + t.Errorf("parsed %d rows, want %d", len(got), batchMaxRows) + } + + overMax := "[" + strings.Join(append(rows, `{"name":"one too many"}`), ",") + "]" + if _, err := parseBatchRows(strings.NewReader(overMax)); err == nil { + t.Errorf("parseBatchRows accepted %d rows, want a cap at %d", batchMaxRows+1, batchMaxRows) + } +} + +// ---- create batch ---- + +func TestItemCreateBatch_happyPath(t *testing.T) { + rec, _ := newGQLRecorder(t, func(op string, vars map[string]any, n int) string { + if op != "ItemCreate" { + t.Errorf("unexpected op %q", op) + } + name, _ := vars["name"].(string) + return itemCreateBody(fmt.Sprintf("100%d", n), name) + }) + + in := `[{"name":"a","group":"topics"},{"name":"b"},{"name":"c","cols":{"text_9":"hi"}}]` + out, err := execItemCreateStdin(t, in, "--board", "9832181507", "-") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + got := decodeBatch(t, out) + if got.Written != 3 || got.Failed != 0 { + t.Errorf("written=%d failed=%d, want 3/0", got.Written, got.Failed) + } + if got.Verb != "created" { + t.Errorf("verb = %q, want created", got.Verb) + } + if len(got.Items) != 3 { + t.Fatalf("items = %d, want 3", len(got.Items)) + } + if got.Errors == nil { + t.Error("errors is null; it must always be present as an array") + } + if rec.countOp("ItemCreate") != 3 { + t.Errorf("made %d ItemCreate calls, want 3", rec.countOp("ItemCreate")) + } + // No shorthand in the payload, so the board's columns must not be fetched. + if rec.countOp("BoardColumnList") != 0 { + t.Errorf("fetched board columns %d times for a --col-only batch, want 0", rec.countOp("BoardColumnList")) + } + + if cv, _ := rec.varsFor("ItemCreate", 2)["columnValues"].(string); cv != `{"text_9":"hi"}` { + t.Errorf("row 2 columnValues = %q, want the row's cols", cv) + } + if gid, _ := rec.varsFor("ItemCreate", 0)["groupId"].(string); gid != "topics" { + t.Errorf("row 0 groupId = %q, want topics", gid) + } +} + +func TestItemCreateBatch_ndjson(t *testing.T) { + rec, _ := newGQLRecorder(t, func(_ string, vars map[string]any, n int) string { + name, _ := vars["name"].(string) + return itemCreateBody(fmt.Sprintf("200%d", n), name) + }) + + in := "{\"name\":\"a\"}\n{\"name\":\"b\"}\n" + out, err := execItemCreateStdin(t, in, "--board", "9832181507", "-") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := decodeBatch(t, out); got.Written != 2 { + t.Errorf("written = %d, want 2", got.Written) + } + if rec.countOp("ItemCreate") != 2 { + t.Errorf("made %d ItemCreate calls, want 2", rec.countOp("ItemCreate")) + } +} + +// TestItemCreateBatch_shorthandsFetchColumnsOnce is the rate-limit point of the +// design: N rows using shorthands cost one column lookup, not N. +func TestItemCreateBatch_shorthandsFetchColumnsOnce(t *testing.T) { + rec, _ := newGQLRecorder(t, func(op string, vars map[string]any, n int) string { + switch op { + case "BoardColumnList": + return boardColumnsBody() + case "ItemCreate": + name, _ := vars["name"].(string) + return itemCreateBody(fmt.Sprintf("300%d", n), name) + default: + t.Errorf("unexpected op %q", op) + return `{"data":{}}` + } + }) + + in := `[{"name":"a","status":"done","due":"2026-05-10"},{"name":"b","number":42,"checkbox":true},{"name":"c"}]` + out, err := execItemCreateStdin(t, in, "--board", "9832181507", "-") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := decodeBatch(t, out); got.Written != 3 { + t.Errorf("written = %d, want 3", got.Written) + } + if n := rec.countOp("BoardColumnList"); n != 1 { + t.Errorf("fetched board columns %d times, want exactly 1 for the whole batch", n) + } + + cv0, _ := rec.varsFor("ItemCreate", 0)["columnValues"].(string) + var parsed0 map[string]json.RawMessage + if err := json.Unmarshal([]byte(cv0), &parsed0); err != nil { + t.Fatalf("row 0 columnValues is not JSON: %q", cv0) + } + if string(parsed0["status_1"]) != `{"label":"Done"}` { + t.Errorf("row 0 status_1 = %s, want the board's own casing", parsed0["status_1"]) + } + if string(parsed0["date_4"]) != `{"date":"2026-05-10"}` { + t.Errorf("row 0 date_4 = %s", parsed0["date_4"]) + } + + cv1, _ := rec.varsFor("ItemCreate", 1)["columnValues"].(string) + var parsed1 map[string]json.RawMessage + if err := json.Unmarshal([]byte(cv1), &parsed1); err != nil { + t.Fatalf("row 1 columnValues is not JSON: %q", cv1) + } + if string(parsed1["numbers_7"]) != `"42"` { + t.Errorf("row 1 numbers_7 = %s, want a bare JSON number to be accepted", parsed1["numbers_7"]) + } + if string(parsed1["checkbox_2"]) != `{"checked":"true"}` { + t.Errorf("row 1 checkbox_2 = %s, want a bare JSON bool to be accepted", parsed1["checkbox_2"]) + } + + // Row 2 sets nothing, so it must send an empty column_values rather than "{}". + if cv2, _ := rec.varsFor("ItemCreate", 2)["columnValues"].(string); cv2 != "" { + t.Errorf("row 2 columnValues = %q, want empty", cv2) + } +} + +// TestItemCreateBatch_badLabelFailsBeforeAnyWrite: one bad status label must not +// leave the first N items created. Validation happens before the first mutation. +func TestItemCreateBatch_badLabelFailsBeforeAnyWrite(t *testing.T) { + rec, _ := newGQLRecorder(t, func(op string, _ map[string]any, _ int) string { + if op == "BoardColumnList" { + return boardColumnsBody() + } + t.Errorf("unexpected write %q: the batch should have failed validation first", op) + return `{"data":{}}` + }) + + in := `[{"name":"a","status":"done"},{"name":"b","status":"Dunn"}]` + _, err := execItemCreateStdin(t, in, "--board", "9832181507", "-") + if err == nil { + t.Fatal("want an error for an unknown status label") + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } + if !strings.Contains(err.Error(), "row 1") { + t.Errorf("error %q does not name the offending row", err.Error()) + } + if rec.countOp("ItemCreate") != 0 { + t.Errorf("created %d items before failing, want 0", rec.countOp("ItemCreate")) + } +} + +// TestItemCreateBatch_partialFailure is the contract that makes a batch retryable: +// exit 2, and errors[].index names exactly which rows to re-send. +func TestItemCreateBatch_partialFailure(t *testing.T) { + rec, _ := newGQLRecorder(t, func(_ string, vars map[string]any, n int) string { + if n == 1 { + return gqlErrorBody("column not found", "InvalidColumnIdException") + } + name, _ := vars["name"].(string) + return itemCreateBody(fmt.Sprintf("400%d", n), name) + }) + + in := `[{"name":"a"},{"name":"b"},{"name":"c"}]` + out, err := execItemCreateStdin(t, in, "--board", "9832181507", "-") + if err == nil { + t.Fatal("want an error so a partial failure does not exit 0") + } + if got := errs.ToExitCode(err); got != 2 { + t.Errorf("exit code = %d, want 2", got) + } + + got := decodeBatch(t, out) + if got.Written != 2 || got.Failed != 1 { + t.Errorf("written=%d failed=%d, want 2/1", got.Written, got.Failed) + } + if len(got.Errors) != 1 { + t.Fatalf("errors = %d, want 1", len(got.Errors)) + } + if got.Errors[0].Index != 1 { + t.Errorf("failed row index = %d, want 1", got.Errors[0].Index) + } + if got.Errors[0].Code == "" { + t.Error("failed row has no code") + } + if got.Errors[0].Code == string(errs.CodeInternal) { + t.Errorf("failed row code = %s; an API rejection should not be reported as internal", got.Errors[0].Code) + } + if !strings.Contains(got.Errors[0].Message, "column not found") { + t.Errorf("failed row message = %q, want the API's message", got.Errors[0].Message) + } + // All three rows are attempted: one bad row does not abandon the rest. + if rec.countOp("ItemCreate") != 3 { + t.Errorf("made %d ItemCreate calls, want 3", rec.countOp("ItemCreate")) + } +} + +func TestItemCreateBatch_validationErrors(t *testing.T) { + tests := []struct { + name string + in string + }{ + {name: "missing name", in: `[{"cols":{"text_9":"hi"}}]`}, + {name: "blank name", in: `[{"name":" "}]`}, + {name: "id not allowed on create", in: `[{"name":"a","id":"456"}]`}, + {name: "date and due together", in: `[{"name":"a","date":"2026-05-10","due":"2026-06-01"}]`}, + {name: "empty col id", in: `[{"name":"a","cols":{"":"hi"}}]`}, + {name: "second row invalid", in: `[{"name":"a"},{"name":""}]`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rec, _ := newGQLRecorder(t, func(op string, _ map[string]any, _ int) string { + t.Errorf("unexpected request %q for invalid input", op) + return `{"data":{}}` + }) + _, err := execItemCreateStdin(t, tt.in, "--board", "9832181507", "-") + if err == nil { + t.Fatalf("want an error for %s", tt.name) + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } + if len(rec.ops()) != 0 { + t.Errorf("made %d requests for invalid input, want 0", len(rec.ops())) + } + }) + } +} + +// TestItemCreateBatch_flagsRejected: silently ignoring --name in batch mode would +// create N items that all disagree with what the caller typed. +func TestItemCreateBatch_flagsRejected(t *testing.T) { + for _, extra := range [][]string{ + {"--name", "x"}, + {"--col", "text_9=\"hi\""}, + {"--status", "Done"}, + } { + args := append([]string{"--board", "9832181507", "-"}, extra...) + _, err := execItemCreateStdin(t, `[{"name":"a"}]`, args...) + if err == nil { + t.Errorf("want an error for batch mode with %v", extra) + continue + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code for %v = %s, want %s", extra, codeOf(err), errs.CodeUsage) + } + } +} + +func TestItemCreateBatch_parentRejected(t *testing.T) { + _, err := execItemCreateStdin(t, `[{"name":"a"}]`, "--parent", "456", "-") + if err == nil { + t.Fatal("want an error: subitems cannot be created in batch") + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } +} + +// TestItemCreateBatch_dryRunSendsNothing keeps --dry-run honest: it must not need a +// token and must not write. +func TestItemCreateBatch_dryRunSendsNothing(t *testing.T) { + rec, _ := newGQLRecorder(t, func(op string, _ map[string]any, _ int) string { + t.Errorf("dry run made a request: %q", op) + return `{"data":{}}` + }) + + in := `[{"name":"a","cols":{"text_9":"hi"}},{"name":"b"}]` + out, err := execItemCreateStdin(t, in, "--board", "9832181507", "-", "--dry-run") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(rec.ops()) != 0 { + t.Errorf("dry run made %d requests, want 0", len(rec.ops())) + } + + var got batchDryRunOutput + if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &got); err != nil { + t.Fatalf("parse output: %v\nraw: %s", err, out) + } + if !got.DryRun || got.Rows != 2 || len(got.Items) != 2 { + t.Fatalf("dry run output = %+v", got) + } + if got.Items[0].ColumnValues != `{"text_9":"hi"}` { + t.Errorf("row 0 column_values = %q", got.Items[0].ColumnValues) + } + if got.Items[1].Name != "b" { + t.Errorf("row 1 name = %q, want b", got.Items[1].Name) + } +} + +// TestItemCreateBatch_dryRunWithShorthandsResolvesColumns: a dry run is the way to +// check a shorthand resolves, so it does fetch columns — but still writes nothing. +func TestItemCreateBatch_dryRunWithShorthandsResolvesColumns(t *testing.T) { + rec, _ := newGQLRecorder(t, func(op string, _ map[string]any, _ int) string { + if op == "BoardColumnList" { + return boardColumnsBody() + } + t.Errorf("dry run made a write request: %q", op) + return `{"data":{}}` + }) + + out, err := execItemCreateStdin(t, `[{"name":"a","status":"Done"}]`, "--board", "9832181507", "-", "--dry-run") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if n := rec.countOp("BoardColumnList"); n != 1 { + t.Errorf("fetched board columns %d times, want 1", n) + } + if rec.countOp("ItemCreate") != 0 { + t.Error("dry run created an item") + } + if !strings.Contains(out, `status_1`) { + t.Errorf("dry run output does not show the resolved column: %s", out) + } +} + +// TestItemCreateBatch_positionalArgRejected: a bare item name as a positional arg is +// an easy mistake, and silently ignoring it would create nothing useful. +func TestItemCreateBatch_positionalArgRejected(t *testing.T) { + _, err := execItemCreateStdin(t, "", "--board", "9832181507", "My Item") + if err == nil { + t.Fatal("want an error for an unexpected positional argument") + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } +} + +// ---- update batch ---- + +func TestItemUpdateBatch_happyPath(t *testing.T) { + rec, _ := newGQLRecorder(t, func(op string, vars map[string]any, _ int) string { + if op != "ItemUpdate" { + t.Errorf("unexpected op %q", op) + } + id, _ := vars["itemId"].(string) + return itemUpdateBody(id, "renamed") + }) + + in := "{\"id\":\"111\",\"cols\":{\"text_9\":\"hi\"}}\n{\"id\":\"222\",\"name\":\"renamed\"}\n" + out, err := execItemUpdateStdin(t, in, "--board", "9832181507", "-") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + got := decodeBatch(t, out) + if got.Written != 2 || got.Failed != 0 { + t.Errorf("written=%d failed=%d, want 2/0", got.Written, got.Failed) + } + if got.Verb != "updated" { + t.Errorf("verb = %q, want updated", got.Verb) + } + if id, _ := rec.varsFor("ItemUpdate", 0)["itemId"].(string); id != "111" { + t.Errorf("row 0 itemId = %q, want 111", id) + } + // A row's "name" is written through the name column, as the single path does. + if cv, _ := rec.varsFor("ItemUpdate", 1)["columnValues"].(string); cv != `{"name":"renamed"}` { + t.Errorf("row 1 columnValues = %q, want the name column", cv) + } +} + +func TestItemUpdateBatch_validationErrors(t *testing.T) { + tests := []struct { + name string + in string + }{ + {name: "missing id", in: `[{"name":"a"}]`}, + {name: "non-numeric id", in: `[{"id":"abc","name":"a"}]`}, + {name: "nothing to update", in: `[{"id":"111"}]`}, + {name: "second row has no id", in: `[{"id":"111","name":"a"},{"name":"b"}]`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rec, _ := newGQLRecorder(t, func(op string, _ map[string]any, _ int) string { + t.Errorf("unexpected request %q for invalid input", op) + return `{"data":{}}` + }) + _, err := execItemUpdateStdin(t, tt.in, "--board", "9832181507", "-") + if err == nil { + t.Fatalf("want an error for %s", tt.name) + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } + if len(rec.ops()) != 0 { + t.Errorf("made %d requests for invalid input, want 0", len(rec.ops())) + } + }) + } +} + +func TestItemUpdateBatch_partialFailure(t *testing.T) { + newGQLRecorder(t, func(_ string, vars map[string]any, n int) string { + if n == 0 { + return gqlErrorBody("item not found", "ResourceNotFoundException") + } + id, _ := vars["itemId"].(string) + return itemUpdateBody(id, "ok") + }) + + in := `[{"id":"111","name":"a"},{"id":"222","name":"b"}]` + out, err := execItemUpdateStdin(t, in, "--board", "9832181507", "-") + if err == nil { + t.Fatal("want an error so a partial failure does not exit 0") + } + if got := errs.ToExitCode(err); got != 2 { + t.Errorf("exit code = %d, want 2", got) + } + + got := decodeBatch(t, out) + if got.Written != 1 || got.Failed != 1 { + t.Errorf("written=%d failed=%d, want 1/1", got.Written, got.Failed) + } + if len(got.Errors) != 1 { + t.Fatalf("errors = %d, want 1", len(got.Errors)) + } + // The item id is echoed on the failure so a caller can retry by id, not position. + if got.Errors[0].ID != "111" { + t.Errorf("failed row id = %q, want 111", got.Errors[0].ID) + } + if got.Errors[0].Index != 0 { + t.Errorf("failed row index = %d, want 0", got.Errors[0].Index) + } +} + +func TestItemUpdateBatch_flagsRejected(t *testing.T) { + _, err := execItemUpdateStdin(t, `[{"id":"111","name":"a"}]`, "--board", "9832181507", "-", "--name", "x") + if err == nil { + t.Fatal("want an error for batch mode with --name") + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } +} + +// ---- single-item shorthands ---- + +func TestItemCreate_shorthandFlags(t *testing.T) { + rec, _ := newGQLRecorder(t, func(op string, vars map[string]any, _ int) string { + if op == "BoardColumnList" { + return boardColumnsBody() + } + name, _ := vars["name"].(string) + return itemCreateBody("5001", name) + }) + + _, err := execItemCreateStdin(t, "", "--board", "9832181507", "--name", "Ship v1", + "--status", "working on it", "--due", "2026-05-10T09:00", "--number", "3", "--checkbox", "yes") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + cv, _ := rec.varsFor("ItemCreate", 0)["columnValues"].(string) + var parsed map[string]json.RawMessage + if err := json.Unmarshal([]byte(cv), &parsed); err != nil { + t.Fatalf("columnValues is not JSON: %q", cv) + } + want := map[string]string{ + "status_1": `{"label":"Working on it"}`, + "date_4": `{"date":"2026-05-10","time":"09:00:00"}`, + "numbers_7": `"3"`, + "checkbox_2": `{"checked":"true"}`, + } + for col, wantVal := range want { + if string(parsed[col]) != wantVal { + t.Errorf("%s = %s, want %s", col, parsed[col], wantVal) + } + } + // The archived status column must not have been written to. + if _, ok := parsed["status_old"]; ok { + t.Error("wrote to an archived column") + } +} + +// TestItemCreate_shorthandNoColumnOfType: the board fixture has no checkbox column, +// so --checkbox must fail loudly rather than being dropped. +func TestItemCreate_shorthandNoColumnOfType(t *testing.T) { + rec, _ := newGQLRecorder(t, func(op string, _ map[string]any, _ int) string { + if op == "BoardColumnList" { + return boardColumnsBodyTextOnly() + } + t.Errorf("unexpected write %q", op) + return `{"data":{}}` + }) + + _, err := execItemCreateStdin(t, "", "--board", "9832181507", "--name", "x", "--checkbox", "true") + if err == nil { + t.Fatal("want an error: the board has no checkbox column") + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } + if rec.countOp("ItemCreate") != 0 { + t.Error("created an item despite an unresolvable shorthand") + } +} + +// TestItemCreate_shorthandCollidesWithCol keeps an ambiguous write from happening. +func TestItemCreate_shorthandCollidesWithCol(t *testing.T) { + newGQLRecorder(t, func(op string, _ map[string]any, _ int) string { + if op == "BoardColumnList" { + return boardColumnsBody() + } + t.Errorf("unexpected write %q", op) + return `{"data":{}}` + }) + + _, err := execItemCreateStdin(t, "", "--board", "9832181507", "--name", "x", + "--status", "Done", "--col", `status_1={"index":2}`) + if err == nil { + t.Fatal("want an error when --status and --col target one column") + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } +} + +func TestItemCreate_shorthandWithParentRejected(t *testing.T) { + _, err := execItemCreateStdin(t, "", "--parent", "456", "--name", "x", "--status", "Done") + if err == nil { + t.Fatal("want an error: shorthands need --board") + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } +} + +// TestItemCreate_dateAndDueMutuallyExclusive: cobra enforces this, and it must stay +// enforced — the two flags write the same column. +func TestItemCreate_dateAndDueMutuallyExclusive(t *testing.T) { + _, err := execItemCreateStdin(t, "", "--board", "9832181507", "--name", "x", + "--date", "2026-05-10", "--due", "2026-06-01") + if err == nil { + t.Fatal("want an error when both --date and --due are given") + } +} + +// TestItemUpdate_shorthandFlags checks the update path resolves shorthands too, and +// that --name still travels as the name column alongside them. +func TestItemUpdate_shorthandFlags(t *testing.T) { + rec, _ := newGQLRecorder(t, func(op string, vars map[string]any, _ int) string { + if op == "BoardColumnList" { + return boardColumnsBody() + } + id, _ := vars["itemId"].(string) + return itemUpdateBody(id, "x") + }) + + _, err := execItemUpdateStdin(t, "", "111", "--board", "9832181507", "--name", "x", "--status", "Done") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + cv, _ := rec.varsFor("ItemUpdate", 0)["columnValues"].(string) + var parsed map[string]json.RawMessage + if err := json.Unmarshal([]byte(cv), &parsed); err != nil { + t.Fatalf("columnValues is not JSON: %q", cv) + } + if string(parsed["status_1"]) != `{"label":"Done"}` { + t.Errorf("status_1 = %s", parsed["status_1"]) + } + if string(parsed["name"]) != `"x"` { + t.Errorf("name = %s, want \"x\"", parsed["name"]) + } +} + +// TestItemUpdate_shorthandOnlyIsEnough: a shorthand alone must satisfy the +// "nothing to update" guard, which predates shorthands. +func TestItemUpdate_shorthandOnlyIsEnough(t *testing.T) { + rec, _ := newGQLRecorder(t, func(op string, vars map[string]any, _ int) string { + if op == "BoardColumnList" { + return boardColumnsBody() + } + id, _ := vars["itemId"].(string) + return itemUpdateBody(id, "x") + }) + + if _, err := execItemUpdateStdin(t, "", "111", "--board", "9832181507", "--status", "Done"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if rec.countOp("ItemUpdate") != 1 { + t.Errorf("made %d ItemUpdate calls, want 1", rec.countOp("ItemUpdate")) + } +} + +// TestItemCreate_clearTextWithEmptyShorthand: --text "" must reach the API as an +// empty value, not be treated as "flag not set". +func TestItemCreate_clearTextWithEmptyShorthand(t *testing.T) { + rec, _ := newGQLRecorder(t, func(op string, vars map[string]any, _ int) string { + if op == "BoardColumnList" { + return boardColumnsBody() + } + name, _ := vars["name"].(string) + return itemCreateBody("6001", name) + }) + + if _, err := execItemCreateStdin(t, "", "--board", "9832181507", "--name", "x", "--text", ""); err != nil { + t.Fatalf("unexpected error: %v", err) + } + cv, _ := rec.varsFor("ItemCreate", 0)["columnValues"].(string) + if cv != `{"text_9":""}` { + t.Errorf("columnValues = %q, want the text column cleared", cv) + } +} diff --git a/internal/cli/item_shorthand.go b/internal/cli/item_shorthand.go new file mode 100644 index 0000000..f3b601d --- /dev/null +++ b/internal/cli/item_shorthand.go @@ -0,0 +1,230 @@ +package cli + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "sort" + "strings" + + gqlclient "github.com/Khan/genqlient/graphql" + "github.com/spf13/cobra" + + "github.com/mondaycom/mcli/internal/api/gen" + "github.com/mondaycom/mcli/internal/api/items/columns" + "github.com/mondaycom/mcli/internal/errs" +) + +// shorthandSpec maps one typed shorthand flag to the monday column type it writes. +// There is no column id in the flag, so the column is resolved by type against the +// board's live columns; see boardColumnIndex.resolve. +type shorthandSpec struct { + flag string + columnType string + usage string +} + +// shorthandSpecs are the typed column shorthands, in the order they are applied and +// reported. Every columnType here must be encodable by the columns package +// (enforced by TestShorthandSpecs_AreEncodable). +var shorthandSpecs = []shorthandSpec{ + {flag: "text", columnType: "text", usage: `set the board's text column, e.g. --text "Follow up"`}, + {flag: "status", columnType: "status", usage: `set the board's status column by label, e.g. --status Done`}, + {flag: "date", columnType: "date", usage: `set the board's date column, e.g. --date 2026-05-10 or 2026-05-10T14:30`}, + {flag: "due", columnType: "date", usage: `alias for --date`}, + {flag: "number", columnType: "numbers", usage: `set the board's numbers column, e.g. --number 42`}, + {flag: "checkbox", columnType: "checkbox", usage: `set the board's checkbox column, e.g. --checkbox true`}, +} + +// shorthandValue is one shorthand the caller actually supplied. +type shorthandValue struct { + spec shorthandSpec + value string +} + +// encodedShorthand is a shorthand after resolution: which column it landed on and +// the wire JSON written there. +type encodedShorthand struct { + flag string + colID string + title string + value json.RawMessage +} + +// addShorthandFlags declares the typed column shorthands on cmd and returns the +// destinations they parse into, keyed by flag name. +func addShorthandFlags(cmd *cobra.Command) map[string]*string { + vals := make(map[string]*string, len(shorthandSpecs)) + for _, sp := range shorthandSpecs { + v := new(string) + cmd.Flags().StringVar(v, sp.flag, "", sp.usage) + vals[sp.flag] = v + } + // --due is sugar for --date; accepting both would be ambiguous about precedence. + cmd.MarkFlagsMutuallyExclusive("date", "due") + return vals +} + +// setShorthands returns the shorthands the caller passed, in spec order. A flag left +// at its zero value is not "set an empty value" — only Changed counts, so +// `--text ""` still clears a text column. +func setShorthands(cmd *cobra.Command, vals map[string]*string) []shorthandValue { + var out []shorthandValue + for _, sp := range shorthandSpecs { + if cmd.Flags().Changed(sp.flag) { + out = append(out, shorthandValue{spec: sp, value: *vals[sp.flag]}) + } + } + return out +} + +// writableColumn is the subset of a board column needed to write to it. +type writableColumn struct { + id string + title string + colType string + settings string +} + +// boardColumnIndex holds a board's live columns grouped by type. +// +// It is fetched at most once per invocation and never cached across invocations: a +// stale mapping would resolve a shorthand onto a column that has since been deleted +// or retyped, which writes the right value to the wrong place. One extra request is +// the cheaper mistake. +type boardColumnIndex struct { + byType map[string][]writableColumn + // types is the sorted set of types present, for "the board has none" errors. + types []string +} + +// fetchBoardColumnIndex reads the board's columns. Archived columns are skipped: +// they cannot be written and would only create phantom ambiguity. +func fetchBoardColumnIndex(ctx context.Context, gql gqlclient.Client, boardID string) (*boardColumnIndex, error) { + resp, err := gen.BoardColumnList(ctx, gql, boardID) + if err != nil { + return nil, err + } + if len(resp.Boards) == 0 { + return nil, errs.NotFound("board %s", boardID) + } + + idx := &boardColumnIndex{byType: make(map[string][]writableColumn)} + for _, c := range resp.Boards[0].Columns { + if c.Archived { + continue + } + t := string(c.Type) + idx.byType[t] = append(idx.byType[t], writableColumn{ + id: c.Id, title: c.Title, colType: t, settings: c.Settings_str, + }) + } + for t := range idx.byType { + idx.types = append(idx.types, t) + } + sort.Strings(idx.types) + return idx, nil +} + +// resolve returns the board's single column of the spec's type. +// +// It never guesses. Silently taking the first of three status columns would move the +// wrong field on a CRM board, and an LLM caller would have no way to notice. +func (idx *boardColumnIndex) resolve(sp shorthandSpec) (writableColumn, error) { + candidates := idx.byType[sp.columnType] + switch len(candidates) { + case 1: + return candidates[0], nil + case 0: + types := "none" + if len(idx.types) > 0 { + types = strings.Join(idx.types, ", ") + } + return writableColumn{}, errs.Usage("--%s needs a %s column, but this board has none; its column types are: %s", + sp.flag, sp.columnType, types) + default: + parts := make([]string, 0, len(candidates)) + for _, c := range candidates { + parts = append(parts, fmt.Sprintf("%s (%q)", c.id, c.title)) + } + return writableColumn{}, errs.Usage("--%s is ambiguous: this board has %d %s columns: %s — use --col = to pick one", + sp.flag, len(candidates), sp.columnType, strings.Join(parts, ", ")) + } +} + +// encodeShorthands resolves each shorthand against the board's columns and encodes +// its value into monday's wire shape. +func encodeShorthands(idx *boardColumnIndex, set []shorthandValue) ([]encodedShorthand, error) { + out := make([]encodedShorthand, 0, len(set)) + seen := make(map[string]string, len(set)) + + for _, sv := range set { + col, err := idx.resolve(sv.spec) + if err != nil { + return nil, err + } + if prev, dup := seen[col.id]; dup { + return nil, errs.Usage("--%s and --%s both target column %s; use only one", prev, sv.spec.flag, col.id) + } + seen[col.id] = sv.spec.flag + + value, err := columns.Encode(sv.spec.columnType, col.settings, sv.value) + if err != nil { + return nil, annotateShorthandError(sv.spec.flag, col, err) + } + + out = append(out, encodedShorthand{flag: sv.spec.flag, colID: col.id, title: col.title, value: value}) + } + return out, nil +} + +// annotateShorthandError re-points an encoder error at the flag and column the caller +// can actually see. The encoder only knows the column type. +func annotateShorthandError(flag string, col writableColumn, err error) error { + msg := err.Error() + if e, ok := errors.AsType[*errs.Error](err); ok { + msg = e.Message + } + return errs.Usage("--%s (column %s %q): %s", flag, col.id, col.title, msg) +} + +// mergeShorthands folds resolved shorthands into a --col map. +// +// A collision is an error rather than a precedence rule: if --status and +// --col status_1=… both target one column, silently letting either win is how you +// ship a write that does not match what the caller wrote. +func mergeShorthands(cols map[string]json.RawMessage, enc []encodedShorthand) error { + for _, e := range enc { + if _, exists := cols[e.colID]; exists { + return errs.Usage("--%s and --col %s both target column %s (%q); use only one", + e.flag, e.colID, e.colID, e.title) + } + cols[e.colID] = e.value + } + return nil +} + +// applyShorthands resolves and merges shorthands into cols, fetching the board's +// columns only when at least one shorthand is set — a pure --col invocation stays a +// single request, exactly as before. +func applyShorthands( + ctx context.Context, gql gqlclient.Client, boardID string, + cols map[string]json.RawMessage, set []shorthandValue, +) ([]encodedShorthand, error) { + if len(set) == 0 { + return nil, nil + } + idx, err := fetchBoardColumnIndex(ctx, gql, boardID) + if err != nil { + return nil, err + } + enc, err := encodeShorthands(idx, set) + if err != nil { + return nil, err + } + if err := mergeShorthands(cols, enc); err != nil { + return nil, err + } + return enc, nil +} diff --git a/internal/cli/item_shorthand_test.go b/internal/cli/item_shorthand_test.go new file mode 100644 index 0000000..c7897ba --- /dev/null +++ b/internal/cli/item_shorthand_test.go @@ -0,0 +1,238 @@ +package cli + +import ( + "encoding/json" + "errors" + "strings" + "testing" + + "github.com/mondaycom/mcli/internal/api/items/columns" + "github.com/mondaycom/mcli/internal/errs" +) + +// statusSettings is a status column's settings_str as monday reports it. +const testStatusSettings = `{"labels":{"0":"Not Started","1":"Working on it","2":"Done"}}` + +// codeOf returns an error's errs code, or "" if it is not an *errs.Error. +func codeOf(err error) errs.Code { + if e, ok := errors.AsType[*errs.Error](err); ok { + return e.Code + } + return "" +} + +// testIndex builds a boardColumnIndex without going through the API. +func testIndex(cols ...writableColumn) *boardColumnIndex { + idx := &boardColumnIndex{byType: map[string][]writableColumn{}} + for _, c := range cols { + idx.byType[c.colType] = append(idx.byType[c.colType], c) + } + for t := range idx.byType { + idx.types = append(idx.types, t) + } + return idx +} + +// specFor returns the shorthandSpec for a flag name. +func specFor(t *testing.T, flag string) shorthandSpec { + t.Helper() + for _, sp := range shorthandSpecs { + if sp.flag == flag { + return sp + } + } + t.Fatalf("no shorthand spec for --%s", flag) + return shorthandSpec{} +} + +// TestShorthandSpecs_AreEncodable guards the contract in shorthandSpecs' doc comment: +// a flag whose column type has no encoder would fail at runtime, not at build time. +func TestShorthandSpecs_AreEncodable(t *testing.T) { + encodable := map[string]bool{} + for _, ct := range columns.EncodableTypes() { + encodable[ct] = true + } + for _, sp := range shorthandSpecs { + if !encodable[sp.columnType] { + t.Errorf("--%s writes column type %q, which columns.Encode does not support", sp.flag, sp.columnType) + } + } +} + +func TestBoardColumnIndex_resolveOne(t *testing.T) { + idx := testIndex( + writableColumn{id: "status_1", title: "Stage", colType: "status", settings: testStatusSettings}, + writableColumn{id: "text_9", title: "Notes", colType: "text"}, + ) + got, err := idx.resolve(specFor(t, "status")) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if got.id != "status_1" { + t.Errorf("resolved column = %q, want status_1", got.id) + } +} + +// TestBoardColumnIndex_resolveNone checks the error names the types the board does +// have, so a caller can pick a real column instead of guessing again. +func TestBoardColumnIndex_resolveNone(t *testing.T) { + idx := testIndex( + writableColumn{id: "text_9", title: "Notes", colType: "text"}, + writableColumn{id: "people_2", title: "Owner", colType: "people"}, + ) + _, err := idx.resolve(specFor(t, "status")) + if err == nil { + t.Fatal("want an error when the board has no status column") + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } + for _, want := range []string{"text", "people"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not name existing type %q", err.Error(), want) + } + } +} + +// TestBoardColumnIndex_resolveAmbiguous is the important one: guessing here would +// write a correct value to the wrong column, silently. +func TestBoardColumnIndex_resolveAmbiguous(t *testing.T) { + idx := testIndex( + writableColumn{id: "status_1", title: "Stage", colType: "status", settings: testStatusSettings}, + writableColumn{id: "status_2", title: "Priority", colType: "status", settings: testStatusSettings}, + ) + _, err := idx.resolve(specFor(t, "status")) + if err == nil { + t.Fatal("want an error when the board has two status columns") + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } + for _, want := range []string{"status_1", "Stage", "status_2", "Priority", "--col"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err.Error(), want) + } + } +} + +func TestEncodeShorthands(t *testing.T) { + idx := testIndex( + writableColumn{id: "status_1", title: "Stage", colType: "status", settings: testStatusSettings}, + writableColumn{id: "date_4", title: "Due", colType: "date"}, + writableColumn{id: "numbers_7", title: "Estimate", colType: "numbers"}, + ) + + set := []shorthandValue{ + {spec: specFor(t, "status"), value: "done"}, + {spec: specFor(t, "due"), value: "2026-05-10"}, + {spec: specFor(t, "number"), value: "3.5"}, + } + + enc, err := encodeShorthands(idx, set) + if err != nil { + t.Fatalf("encodeShorthands: %v", err) + } + if len(enc) != 3 { + t.Fatalf("encoded %d shorthands, want 3", len(enc)) + } + + byCol := map[string]string{} + for _, e := range enc { + byCol[e.colID] = string(e.value) + } + want := map[string]string{ + "status_1": `{"label":"Done"}`, + "date_4": `{"date":"2026-05-10"}`, + "numbers_7": `"3.5"`, + } + for col, wantVal := range want { + if byCol[col] != wantVal { + t.Errorf("column %s = %s, want %s", col, byCol[col], wantVal) + } + } +} + +// TestEncodeShorthands_badLabelNamesFlagAndColumn checks the encoder's error is +// re-pointed at what the caller typed. "status: 'Dunn' is not a label" is much less +// useful than naming the flag and the column it resolved to. +func TestEncodeShorthands_badLabelNamesFlagAndColumn(t *testing.T) { + idx := testIndex(writableColumn{id: "status_1", title: "Stage", colType: "status", settings: testStatusSettings}) + _, err := encodeShorthands(idx, []shorthandValue{{spec: specFor(t, "status"), value: "Dunn"}}) + if err == nil { + t.Fatal("want an error for an unknown status label") + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } + for _, want := range []string{"--status", "status_1", "Stage", "Done"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err.Error(), want) + } + } +} + +// TestEncodeShorthands_twoFlagsOneColumn covers --date and --due both landing on the +// board's only date column when the mutual-exclusion check is bypassed (batch rows go +// through their own validation, so this layer must still refuse). +func TestEncodeShorthands_twoFlagsOneColumn(t *testing.T) { + idx := testIndex(writableColumn{id: "date_4", title: "Due", colType: "date"}) + _, err := encodeShorthands(idx, []shorthandValue{ + {spec: specFor(t, "date"), value: "2026-05-10"}, + {spec: specFor(t, "due"), value: "2026-06-01"}, + }) + if err == nil { + t.Fatal("want an error when two shorthands target one column") + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } +} + +func TestMergeShorthands(t *testing.T) { + cols := map[string]json.RawMessage{"text_9": json.RawMessage(`"hi"`)} + err := mergeShorthands(cols, []encodedShorthand{ + {flag: "status", colID: "status_1", title: "Stage", value: json.RawMessage(`{"label":"Done"}`)}, + }) + if err != nil { + t.Fatalf("mergeShorthands: %v", err) + } + if string(cols["status_1"]) != `{"label":"Done"}` { + t.Errorf("status_1 = %s, want the encoded label", cols["status_1"]) + } + if string(cols["text_9"]) != `"hi"` { + t.Errorf("mergeShorthands clobbered an existing --col value") + } +} + +// TestMergeShorthands_collision: --status and --col status_1=... in one command is +// ambiguous. Picking a winner would send a value the caller did not ask for. +func TestMergeShorthands_collision(t *testing.T) { + cols := map[string]json.RawMessage{"status_1": json.RawMessage(`{"index":2}`)} + err := mergeShorthands(cols, []encodedShorthand{ + {flag: "status", colID: "status_1", title: "Stage", value: json.RawMessage(`{"label":"Done"}`)}, + }) + if err == nil { + t.Fatal("want an error when a shorthand and --col target one column") + } + if codeOf(err) != errs.CodeUsage { + t.Errorf("code = %s, want %s", codeOf(err), errs.CodeUsage) + } + for _, want := range []string{"--status", "--col", "status_1"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err.Error(), want) + } + } +} + +// TestApplyShorthands_noneSkipsFetch: a pure --col command must not gain a request. +// A nil client would panic if applyShorthands tried to use it. +func TestApplyShorthands_noneSkipsFetch(t *testing.T) { + cols := map[string]json.RawMessage{"text_9": json.RawMessage(`"hi"`)} + enc, err := applyShorthands(t.Context(), nil, "123", cols, nil) + if err != nil { + t.Fatalf("applyShorthands: %v", err) + } + if len(enc) != 0 { + t.Errorf("encoded %d shorthands, want 0", len(enc)) + } +} From 3d46290e83ae4452c43d4327ed5ceb0f161ae61e Mon Sep 17 00:00:00 2001 From: Arnon Rotem-Gal-Oz Date: Tue, 8 Sep 2026 13:44:29 +0300 Subject: [PATCH 4/6] feat(api): schema staleness reporting and refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every 'mcli api' operation is generated from the embedded schema, so a stale schema silently misrepresents the API surface — an operation that exists reads as missing. 'mcli api' now warns on stderr (never stdout, which carries the JSON contract) when the schema is old, and 'mcli schema status' / 'mcli schema refresh' report which schema is in use and replace it. The embed carries a fetched_at stamp so age is a fact rather than a guess, and 'config set api-version' reuses the shared fetch-and-cache path instead of duplicating token resolution and file writing. Co-Authored-By: Claude Opus 5 --- internal/api/schema/schema.go | 25 ++ internal/cli/api.go | 5 + internal/cli/config.go | 19 +- internal/cli/root.go | 1 + internal/cli/schema.go | 339 ++++++++++++++++++++++++++ internal/cli/schema_test.go | 433 ++++++++++++++++++++++++++++++++++ schema/embed.go | 26 +- schema/fetched_at.txt | 1 + tools/introspect/main.go | 16 +- 9 files changed, 843 insertions(+), 22 deletions(-) create mode 100644 internal/cli/schema.go create mode 100644 internal/cli/schema_test.go create mode 100644 schema/fetched_at.txt diff --git a/internal/api/schema/schema.go b/internal/api/schema/schema.go index bf4fd6a..dac30f1 100644 --- a/internal/api/schema/schema.go +++ b/internal/api/schema/schema.go @@ -33,12 +33,21 @@ type ArgDef struct { DefaultValue string } +// Source values reported by LoadedSource. +const ( + // SourceCached means a locally refreshed schema.graphql was parsed and used. + SourceCached = "cached" + // SourceEmbedded means the SDL compiled into the binary was used. + SourceEmbedded = "embedded" +) + var ( mu sync.Mutex cachedSchema *ast.Schema cacheErr error cacheDir string cacheLoaded bool + loadedSource string ) // SetConfigDir sets the directory used to locate a cached schema.graphql file. @@ -79,6 +88,8 @@ func Load() (*ast.Schema, error) { s, err = gqlparser.LoadSchema(src) if err != nil { err = fmt.Errorf("parse local schema: %w", err) + } else { + loadedSource = SourceCached } } } @@ -89,6 +100,7 @@ func Load() (*ast.Schema, error) { if err != nil { err = fmt.Errorf("parse schema: %w", err) } + loadedSource = SourceEmbedded } cachedSchema = s @@ -98,6 +110,19 @@ func Load() (*ast.Schema, error) { return cachedSchema, cacheErr } +// LoadedSource reports which SDL Load actually parsed: SourceCached or +// SourceEmbedded. It matters because a cached schema.graphql that exists but +// fails to parse is silently ignored in favour of the embedded copy, so the +// presence of the file is not proof that it is in use. +func LoadedSource() (string, error) { + if _, err := Load(); err != nil { + return "", err + } + mu.Lock() + defer mu.Unlock() + return loadedSource, nil +} + // QueryFields returns FieldDef for each field on the Query type. func QueryFields() ([]FieldDef, error) { s, err := Load() diff --git a/internal/cli/api.go b/internal/cli/api.go index 2363c63..69c13ca 100644 --- a/internal/cli/api.go +++ b/internal/cli/api.go @@ -56,6 +56,11 @@ Example: --arg column_id=status --arg value='{"label":"Done"}'`, Args: cobra.ArbitraryArgs, DisableFlagParsing: false, + // Every 'api' operation is generated from the schema, so a stale schema + // silently misrepresents the API surface. Warn on stderr, never stdout. + PersistentPreRun: func(cmd *cobra.Command, _ []string) { + warnIfSchemaStale(cmd) + }, RunE: func(cmd *cobra.Command, args []string) error { if len(args) == 0 { return errs.Usage("provide an operation name, or use 'mcli api list'") diff --git a/internal/cli/config.go b/internal/cli/config.go index 978aaaa..5487fa0 100644 --- a/internal/cli/config.go +++ b/internal/cli/config.go @@ -10,7 +10,6 @@ import ( apischema "github.com/mondaycom/mcli/internal/api/schema" "github.com/mondaycom/mcli/internal/config" "github.com/mondaycom/mcli/internal/errs" - "github.com/mondaycom/mcli/internal/secrets" ) // apiVersionRE accepts YYYY-MM calendar versions (e.g. 2026-07) or named @@ -129,14 +128,7 @@ func runConfigSetAPIVersion(cmd *cobra.Command, cfgPath string, cfg config.Confi } // Resolve token to fetch the schema. Missing token is non-fatal: warn and return. - var store config.Store - if cfg.SecretStore != "" { - st, openErr := secrets.Open(cfg.SecretStore, resolveConfigDir()) - if openErr == nil { - store = st - } - } - token, tokenErr := config.ResolveToken(cfg, globals.Token, store) + token, tokenErr := resolveAPIToken(cfg) if tokenErr != nil { _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "api-version saved; run 'mcli auth login' then 'mcli config set api-version %s' to refresh schema\n", @@ -144,7 +136,7 @@ func runConfigSetAPIVersion(cmd *cobra.Command, cfgPath string, cfg config.Confi return nil } - sdl, fetchErr := apischema.FetchSchema(cmd.Context(), token, config.ResolveEndpoint(cfg), value) + schemaPath, fetchErr := fetchAndCacheSchema(cmd.Context(), cfg, value, token) if fetchErr != nil { // Revert the saved version. cfg.APIVersion = "" @@ -155,13 +147,6 @@ func runConfigSetAPIVersion(cmd *cobra.Command, cfgPath string, cfg config.Confi value, fetchErr, config.ResolveAPIVersion(config.Config{})) } - schemaPath := apischema.CachedSchemaPath(resolveConfigDir()) - if err := os.WriteFile(schemaPath, []byte(sdl), 0o600); err != nil { - return errs.Internal("write schema cache: %v", err) - } - - apischema.SetConfigDir(resolveConfigDir()) - _, err := fmt.Fprintf(cmd.OutOrStdout(), "fetched schema for %s → %s\n", value, schemaPath) return err } diff --git a/internal/cli/root.go b/internal/cli/root.go index d58a9dd..76bdb81 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -64,6 +64,7 @@ func init() { rootCmd.AddCommand(newSearchCmd()) rootCmd.AddCommand(newDocCmd()) rootCmd.AddCommand(newAPICmd()) + rootCmd.AddCommand(newSchemaCmd()) requireKnownSubcommands(rootCmd) diff --git a/internal/cli/schema.go b/internal/cli/schema.go new file mode 100644 index 0000000..096f0ff --- /dev/null +++ b/internal/cli/schema.go @@ -0,0 +1,339 @@ +package cli + +import ( + "context" + "encoding/json" + "fmt" + "os" + "sort" + "time" + + "github.com/spf13/cobra" + + apischema "github.com/mondaycom/mcli/internal/api/schema" + "github.com/mondaycom/mcli/internal/config" + "github.com/mondaycom/mcli/internal/errs" + "github.com/mondaycom/mcli/internal/secrets" + rawschema "github.com/mondaycom/mcli/schema" +) + +// staleAfterDays is how old the live schema may get before mcli warns. monday +// ships API versions quarterly and adds fields within a version continuously, so +// a month is a reasonable "you are probably missing operations" threshold. +const staleAfterDays = 30 + +// maxDeltaNames caps how many added/removed type names a refresh prints before +// collapsing to a count, so the output stays readable when a version bump lands. +const maxDeltaNames = 10 + +func newSchemaCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "schema", + Short: "Inspect and refresh the local monday.com GraphQL schema", + Long: `Inspect and refresh the monday.com GraphQL schema that drives 'mcli api'. + +mcli ships with an embedded schema. Refreshing fetches the live schema with your +own API token and caches it locally, so you pick up new monday operations without +waiting for an mcli release.`, + } + cmd.AddCommand(newSchemaRefreshCmd()) + cmd.AddCommand(newSchemaStatusCmd()) + return cmd +} + +func newSchemaRefreshCmd() *cobra.Command { + var apiVersion string + + cmd := &cobra.Command{ + Use: "refresh", + Short: "Fetch the live schema and cache it locally", + Long: `Fetch the monday.com GraphQL schema by introspection and cache it locally. + +Uses your configured API token and API version. The cached schema takes precedence +over the embedded one for all 'mcli api' operations.`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + return runSchemaRefresh(cmd, apiVersion) + }, + } + + cmd.Flags().StringVar(&apiVersion, "api-version", "", + "fetch this API version instead of the configured one (not persisted)") + + return cmd +} + +func newSchemaStatusCmd() *cobra.Command { + return &cobra.Command{ + Use: "status", + Short: "Report which schema is in use and how old it is", + Args: cobra.NoArgs, + RunE: runSchemaStatus, + } +} + +// schemaStatus is the JSON shape for 'mcli schema status'. +type schemaStatus struct { + // Source is "cached" when a locally refreshed schema is in use, "embedded" + // when the copy compiled into the binary is. + Source string `json:"source"` + Path string `json:"path,omitempty"` + APIVersion string `json:"api_version"` + // FetchedAt is YYYY-MM-DD, or empty when the date cannot be determined. + FetchedAt string `json:"fetched_at,omitempty"` + // AgeDays is -1 when FetchedAt is unknown. + AgeDays int `json:"age_days"` + Stale bool `json:"stale"` + Types int `json:"types"` +} + +// liveSchemaInfo reports which schema apischema.Load would use and how old it is. +// A cached schema's age comes from its file mtime; the embedded schema's from the +// date recorded at `make schema` time. +func liveSchemaInfo(cfg config.Config) schemaStatus { + st := schemaStatus{ + Source: apischema.SourceEmbedded, + APIVersion: config.ResolveAPIVersion(cfg), + AgeDays: -1, + } + + if s, err := apischema.Load(); err == nil && s != nil { + st.Types = len(s.Types) + } + // Ask apischema which SDL it parsed rather than inferring it from the file's + // existence: an unparseable cache falls back to the embedded copy. + if src, err := apischema.LoadedSource(); err == nil { + st.Source = src + } + + var fetched time.Time + if st.Source == apischema.SourceCached { + st.Path = apischema.CachedSchemaPath(resolveConfigDir()) + if fi, err := os.Stat(st.Path); err == nil { + fetched = fi.ModTime() + } + } else { + fetched = rawschema.FetchedAt() + } + + if !fetched.IsZero() { + st.FetchedAt = fetched.Format("2006-01-02") + st.AgeDays = int(time.Since(fetched).Hours() / 24) + st.Stale = st.AgeDays > staleAfterDays + } + + return st +} + +// warnIfSchemaStale emits a one-line staleness warning to stderr. +// +// It deliberately never fetches: mcli is driven by LLM agents in steady state, and +// an implicit network call on a read path can hang, fail, or need a token that is +// not there. It also never writes to stdout, which carries the JSON contract. +func warnIfSchemaStale(cmd *cobra.Command) { + cfg, err := config.Load(resolveConfigPath()) + if err != nil { + return + } + st := liveSchemaInfo(cfg) + if !st.Stale { + return + } + // A failed warning is not worth reporting: it must never affect the command's + // own exit status. + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), + "warning: %s monday schema is %d days old; run 'mcli schema refresh' to update\n", + st.Source, st.AgeDays) +} + +// resolveAPIToken resolves the API token with the standard precedence: +// --token flag, then MONDAY_API_TOKEN, then the configured secret store. +func resolveAPIToken(cfg config.Config) (config.APIToken, error) { + var store config.Store + if cfg.SecretStore != "" { + st, openErr := secrets.Open(cfg.SecretStore, resolveConfigDir()) + if openErr != nil { + return "", openErr + } + store = st + } + return config.ResolveToken(cfg, globals.Token, store) +} + +// fetchAndCacheSchema introspects the live schema for apiVersion, writes it to the +// local cache and busts the in-memory cache so the new SDL takes effect +// immediately. It is the single implementation shared by 'mcli schema refresh' and +// 'mcli config set api-version'. +func fetchAndCacheSchema(ctx context.Context, cfg config.Config, apiVersion string, token config.APIToken) (string, error) { + sdl, err := apischema.FetchSchema(ctx, token, config.ResolveEndpoint(cfg), apiVersion) + if err != nil { + return "", err + } + + path := apischema.CachedSchemaPath(resolveConfigDir()) + if err := os.WriteFile(path, []byte(sdl), 0o600); err != nil { + return "", errs.Internal("write schema cache: %v", err) + } + + apischema.SetConfigDir(resolveConfigDir()) + return path, nil +} + +// schemaTypeNames returns the set of type names in the currently loaded schema. +// It returns nil if the schema cannot be loaded, so a refresh still succeeds when +// only the delta summary is unavailable. +func schemaTypeNames() map[string]bool { + s, err := apischema.Load() + if err != nil || s == nil { + return nil + } + names := make(map[string]bool, len(s.Types)) + for name := range s.Types { + names[name] = true + } + return names +} + +// diffNames returns names present in b but not a, sorted. +func diffNames(a, b map[string]bool) []string { + var out []string + for name := range b { + if !a[name] { + out = append(out, name) + } + } + sort.Strings(out) + return out +} + +// schemaRefreshOutput is the JSON shape for 'mcli schema refresh'. +type schemaRefreshOutput struct { + Path string `json:"path"` + APIVersion string `json:"api_version"` + Types int `json:"types"` + Added []string `json:"added"` + Removed []string `json:"removed"` +} + +func runSchemaRefresh(cmd *cobra.Command, apiVersionOverride string) error { + cfg, err := config.Load(resolveConfigPath()) + if err != nil { + return errs.Internal("load config: %v", err) + } + + apiVersion := config.ResolveAPIVersion(cfg) + if apiVersionOverride != "" { + if !apiVersionRE.MatchString(apiVersionOverride) { + return errs.Usage("invalid api-version %q: must be YYYY-MM (e.g. 2026-07) or a named version (e.g. dev)", apiVersionOverride) + } + apiVersion = apiVersionOverride + } + + token, err := resolveAPIToken(cfg) + if err != nil { + return err + } + + before := schemaTypeNames() + + path, err := fetchAndCacheSchema(cmd.Context(), cfg, apiVersion, token) + if err != nil { + return err + } + + after := schemaTypeNames() + + out := schemaRefreshOutput{ + Path: path, + APIVersion: apiVersion, + Types: len(after), + Added: diffNames(before, after), + Removed: diffNames(after, before), + } + + mode, modeErr := resolveOutputMode(os.Stdout, globals, configOutputMode()) + if modeErr != nil { + return modeErr + } + + if mode == ModeJSON { + data, mErr := json.Marshal(out) + if mErr != nil { + return errs.Internal("marshal output: %v", mErr) + } + _, err = fmt.Fprintln(cmd.OutOrStdout(), string(data)) + return err + } + + o := cmd.OutOrStdout() + _, _ = fmt.Fprintf(o, "fetched schema for %s → %s\n", apiVersion, path) + _, _ = fmt.Fprintf(o, "types: %d\n", out.Types) + printDelta(o, "added", out.Added) + printDelta(o, "removed", out.Removed) + if len(out.Added) == 0 && len(out.Removed) == 0 && before != nil { + _, _ = fmt.Fprintln(o, "no type changes") + } + return nil +} + +// printDelta writes a "label: a, b, c" line, collapsing to a count past +// maxDeltaNames. It prints nothing for an empty delta. +func printDelta(w interface{ Write([]byte) (int, error) }, label string, names []string) { + if len(names) == 0 { + return + } + if len(names) > maxDeltaNames { + _, _ = fmt.Fprintf(w, "%s: %d types\n", label, len(names)) + return + } + for _, n := range names { + _, _ = fmt.Fprintf(w, "%s: %s\n", label, n) + } +} + +func runSchemaStatus(cmd *cobra.Command, _ []string) error { + cfg, err := config.Load(resolveConfigPath()) + if err != nil { + return errs.Internal("load config: %v", err) + } + + st := liveSchemaInfo(cfg) + + mode, modeErr := resolveOutputMode(os.Stdout, globals, configOutputMode()) + if modeErr != nil { + return modeErr + } + + switch mode { + case ModeJSON: + data, mErr := json.Marshal(st) + if mErr != nil { + return errs.Internal("marshal output: %v", mErr) + } + _, err = fmt.Fprintln(cmd.OutOrStdout(), string(data)) + return err + + case ModeTerse: + _, err = fmt.Fprintf(cmd.OutOrStdout(), "%s %s age=%dd types=%d stale=%t\n", + st.Source, st.APIVersion, st.AgeDays, st.Types, st.Stale) + return err + + default: + o := cmd.OutOrStdout() + _, _ = fmt.Fprintf(o, "source: %s\n", st.Source) + if st.Path != "" { + _, _ = fmt.Fprintf(o, "path: %s\n", st.Path) + } + _, _ = fmt.Fprintf(o, "api-version: %s\n", st.APIVersion) + if st.FetchedAt != "" { + _, _ = fmt.Fprintf(o, "fetched: %s (%d days ago)\n", st.FetchedAt, st.AgeDays) + } else { + _, _ = fmt.Fprintf(o, "fetched: unknown\n") + } + _, _ = fmt.Fprintf(o, "types: %d\n", st.Types) + if st.Stale { + _, _ = fmt.Fprintf(o, "\nschema is stale; run 'mcli schema refresh' to update\n") + } + return nil + } +} diff --git a/internal/cli/schema_test.go b/internal/cli/schema_test.go new file mode 100644 index 0000000..4b44e63 --- /dev/null +++ b/internal/cli/schema_test.go @@ -0,0 +1,433 @@ +package cli + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/spf13/cobra" + + apischema "github.com/mondaycom/mcli/internal/api/schema" + "github.com/mondaycom/mcli/internal/errs" + rawschema "github.com/mondaycom/mcli/schema" +) + +// runSchemaCmd executes a 'schema' subcommand against an isolated config dir. +// Not safe for parallel tests: it mutates globals and the apischema cache. +func runSchemaCmd(t *testing.T, dir string, args ...string) (string, string, error) { + t.Helper() + + origGlobals := globals + t.Cleanup(func() { globals = origGlobals }) + globals.Config = filepath.Join(dir, "config.yaml") + // Default to JSON so assertions don't depend on whether the test binary's + // stdout happens to be a TTY. A caller that pre-sets another mode keeps it. + if !globals.Pretty && !globals.Terse && !globals.CSV { + globals.JSON = true + } + + apischema.SetConfigDir(dir) + t.Cleanup(func() { apischema.SetConfigDir("") }) + + root := &cobra.Command{Use: "mcli", SilenceErrors: true, SilenceUsage: true} + root.AddCommand(newSchemaCmd()) + + var out, errOut bytes.Buffer + root.SetOut(&out) + root.SetErr(&errOut) + root.SetArgs(args) + err := root.Execute() + return out.String(), errOut.String(), err +} + +// introspectionServer serves a minimal but valid introspection response. +func introspectionServer(t *testing.T) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, minimalIntrospectionJSONForCLI()) + })) + t.Cleanup(srv.Close) + + origFetch := apischema.FetchHTTPClient + apischema.FetchHTTPClient = srv.Client() + t.Cleanup(func() { apischema.FetchHTTPClient = origFetch }) + + t.Setenv("MONDAY_API_URL", srv.URL) + return srv +} + +func decodeStatus(t *testing.T, out string) schemaStatus { + t.Helper() + var st schemaStatus + if err := json.Unmarshal([]byte(out), &st); err != nil { + t.Fatalf("unmarshal status %q: %v", out, err) + } + return st +} + +// TestSchemaStatus_embedded verifies status reports the embedded SDL when no +// local cache exists, dated from the provenance file written by 'make schema'. +func TestSchemaStatus_embedded(t *testing.T) { + dir := t.TempDir() + + out, _, err := runSchemaCmd(t, dir, "schema", "status") + if err != nil { + t.Fatalf("schema status: %v", err) + } + + st := decodeStatus(t, out) + if st.Source != apischema.SourceEmbedded { + t.Errorf("source = %q, want %q", st.Source, apischema.SourceEmbedded) + } + if st.Path != "" { + t.Errorf("path = %q, want empty for the embedded schema", st.Path) + } + if st.Types < 100 { + t.Errorf("types = %d, want the full embedded schema (>100)", st.Types) + } + if st.APIVersion == "" { + t.Error("api_version is empty") + } + if want := rawschema.FetchedAt().Format("2006-01-02"); st.FetchedAt != want { + t.Errorf("fetched_at = %q, want %q from schema/fetched_at.txt", st.FetchedAt, want) + } + // Don't hardcode staleness: it depends on today's date. Assert the invariant. + if st.Stale != (st.AgeDays > staleAfterDays) { + t.Errorf("stale = %t but age_days = %d (threshold %d)", st.Stale, st.AgeDays, staleAfterDays) + } +} + +// TestSchemaStatus_cached verifies a parseable local cache is reported as the +// live schema, aged by its file mtime. +func TestSchemaStatus_cached(t *testing.T) { + dir := t.TempDir() + writeCachedSchema(t, dir, time.Now()) + + out, _, err := runSchemaCmd(t, dir, "schema", "status") + if err != nil { + t.Fatalf("schema status: %v", err) + } + + st := decodeStatus(t, out) + if st.Source != apischema.SourceCached { + t.Errorf("source = %q, want %q", st.Source, apischema.SourceCached) + } + if st.Path != apischema.CachedSchemaPath(dir) { + t.Errorf("path = %q, want %q", st.Path, apischema.CachedSchemaPath(dir)) + } + if st.AgeDays != 0 { + t.Errorf("age_days = %d, want 0 for a just-written cache", st.AgeDays) + } + if st.Stale { + t.Error("stale = true for a just-written cache") + } +} + +// TestSchemaStatus_cachedStale verifies the staleness threshold is driven by the +// cache file's mtime. +func TestSchemaStatus_cachedStale(t *testing.T) { + dir := t.TempDir() + writeCachedSchema(t, dir, time.Now().AddDate(0, 0, -(staleAfterDays+5))) + + out, _, err := runSchemaCmd(t, dir, "schema", "status") + if err != nil { + t.Fatalf("schema status: %v", err) + } + + st := decodeStatus(t, out) + if !st.Stale { + t.Errorf("stale = false for a schema %d days old", st.AgeDays) + } + if st.AgeDays <= staleAfterDays { + t.Errorf("age_days = %d, want > %d", st.AgeDays, staleAfterDays) + } +} + +// TestSchemaStatus_unparseableCacheReportsEmbedded guards the one case where the +// cache file's existence lies: apischema silently falls back to the embedded SDL +// when the cached file doesn't parse, and status must say so. +func TestSchemaStatus_unparseableCacheReportsEmbedded(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(apischema.CachedSchemaPath(dir), []byte("this is not graphql {{{"), 0o600); err != nil { + t.Fatalf("write cache: %v", err) + } + + out, _, err := runSchemaCmd(t, dir, "schema", "status") + if err != nil { + t.Fatalf("schema status: %v", err) + } + + st := decodeStatus(t, out) + if st.Source != apischema.SourceEmbedded { + t.Errorf("source = %q, want %q when the cache doesn't parse", st.Source, apischema.SourceEmbedded) + } + if st.Types < 100 { + t.Errorf("types = %d, want the embedded schema to be in use", st.Types) + } +} + +// TestSchemaStatus_terse verifies the single-line form stays greppable. +func TestSchemaStatus_terse(t *testing.T) { + dir := t.TempDir() + writeCachedSchema(t, dir, time.Now()) + + origGlobals := globals + t.Cleanup(func() { globals = origGlobals }) + globals.Terse = true + + out, _, err := runSchemaCmd(t, dir, "schema", "status") + if err != nil { + t.Fatalf("schema status: %v", err) + } + + line := strings.TrimSpace(out) + if strings.Contains(line, "\n") { + t.Errorf("terse output is not a single line: %q", out) + } + for _, want := range []string{apischema.SourceCached, "age=0d", "stale=false", "types="} { + if !strings.Contains(line, want) { + t.Errorf("terse output %q is missing %q", line, want) + } + } +} + +// TestSchemaRefresh_success verifies refresh writes the cache, reports the type +// delta, and flips status over to the cached schema. +func TestSchemaRefresh_success(t *testing.T) { + dir := t.TempDir() + introspectionServer(t) + + origGlobals := globals + t.Cleanup(func() { globals = origGlobals }) + globals.Token = "fake-token" + + out, _, err := runSchemaCmd(t, dir, "schema", "refresh") + if err != nil { + t.Fatalf("schema refresh: %v", err) + } + + var got schemaRefreshOutput + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("unmarshal refresh output %q: %v", out, err) + } + if got.Path != apischema.CachedSchemaPath(dir) { + t.Errorf("path = %q, want %q", got.Path, apischema.CachedSchemaPath(dir)) + } + if got.APIVersion == "" { + t.Error("api_version is empty") + } + if got.Types == 0 { + t.Error("types = 0, want the refreshed schema's type count") + } + // The fixture schema is far smaller than the embedded one, so the delta must + // show the embedded types dropping out. + if len(got.Removed) == 0 { + t.Error("removed is empty, want the embedded types absent from the fixture") + } + + data, readErr := os.ReadFile(apischema.CachedSchemaPath(dir)) + if readErr != nil { + t.Fatalf("cache not written: %v", readErr) + } + if !strings.Contains(string(data), "type Query") { + t.Errorf("cache missing 'type Query': %s", data) + } + + statusOut, _, statusErr := runSchemaCmd(t, dir, "schema", "status") + if statusErr != nil { + t.Fatalf("schema status: %v", statusErr) + } + if st := decodeStatus(t, statusOut); st.Source != apischema.SourceCached { + t.Errorf("after refresh source = %q, want %q", st.Source, apischema.SourceCached) + } +} + +// TestSchemaRefresh_apiVersionOverrideNotPersisted verifies --api-version affects +// the fetch without writing to config.yaml. +func TestSchemaRefresh_apiVersionOverrideNotPersisted(t *testing.T) { + dir := t.TempDir() + + var gotVersion string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotVersion = r.Header.Get("API-Version") + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, minimalIntrospectionJSONForCLI()) + })) + defer srv.Close() + + origFetch := apischema.FetchHTTPClient + apischema.FetchHTTPClient = srv.Client() + t.Cleanup(func() { apischema.FetchHTTPClient = origFetch }) + t.Setenv("MONDAY_API_URL", srv.URL) + + origGlobals := globals + t.Cleanup(func() { globals = origGlobals }) + globals.Token = "fake-token" + + out, _, err := runSchemaCmd(t, dir, "schema", "refresh", "--api-version", "2026-08") + if err != nil { + t.Fatalf("schema refresh: %v", err) + } + + if gotVersion != "2026-08" { + t.Errorf("API-Version header = %q, want 2026-08", gotVersion) + } + var got schemaRefreshOutput + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("unmarshal refresh output: %v", err) + } + if got.APIVersion != "2026-08" { + t.Errorf("api_version = %q, want 2026-08", got.APIVersion) + } + if _, statErr := os.Stat(filepath.Join(dir, "config.yaml")); statErr == nil { + t.Error("refresh wrote config.yaml; --api-version must not be persisted") + } +} + +// TestSchemaRefresh_invalidAPIVersion verifies the override is validated before +// any network call, so a typo is a usage error rather than an API error. +func TestSchemaRefresh_invalidAPIVersion(t *testing.T) { + dir := t.TempDir() + + _, _, err := runSchemaCmd(t, dir, "schema", "refresh", "--api-version", "2026-7") + if err == nil { + t.Fatal("expected an error for a malformed api-version") + } + e, ok := err.(*errs.Error) + if !ok || e.Code != errs.CodeUsage { + t.Errorf("error = %T %v, want errs.CodeUsage", err, err) + } +} + +// TestSchemaRefresh_noToken verifies refresh fails with an auth error rather than +// silently leaving the schema alone. +func TestSchemaRefresh_noToken(t *testing.T) { + dir := t.TempDir() + t.Setenv("MONDAY_API_TOKEN", "") + + origGlobals := globals + t.Cleanup(func() { globals = origGlobals }) + globals.Token = "" + + _, _, err := runSchemaCmd(t, dir, "schema", "refresh") + if err == nil { + t.Fatal("expected an error when no token is available") + } + e, ok := err.(*errs.Error) + if !ok || e.Code != errs.CodeAuth { + t.Errorf("error = %T %v, want errs.CodeAuth", err, err) + } + if _, statErr := os.Stat(apischema.CachedSchemaPath(dir)); statErr == nil { + t.Error("cache was written despite the missing token") + } +} + +// TestSchemaRefresh_fetchError verifies an API failure leaves the existing cache +// untouched. +func TestSchemaRefresh_fetchError(t *testing.T) { + dir := t.TempDir() + writeCachedSchema(t, dir, time.Now()) + before, err := os.ReadFile(apischema.CachedSchemaPath(dir)) + if err != nil { + t.Fatalf("read cache: %v", err) + } + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + })) + defer srv.Close() + + origFetch := apischema.FetchHTTPClient + apischema.FetchHTTPClient = srv.Client() + t.Cleanup(func() { apischema.FetchHTTPClient = origFetch }) + t.Setenv("MONDAY_API_URL", srv.URL) + + origGlobals := globals + t.Cleanup(func() { globals = origGlobals }) + globals.Token = "fake-token" + + if _, _, refreshErr := runSchemaCmd(t, dir, "schema", "refresh"); refreshErr == nil { + t.Fatal("expected an error when introspection returns 403") + } + + after, err := os.ReadFile(apischema.CachedSchemaPath(dir)) + if err != nil { + t.Fatalf("cache disappeared after a failed refresh: %v", err) + } + if !bytes.Equal(before, after) { + t.Error("a failed refresh modified the existing cache") + } +} + +// TestWarnIfSchemaStale_stderrOnly verifies the staleness warning never touches +// stdout, which carries the JSON contract that LLM callers parse. +func TestWarnIfSchemaStale_stderrOnly(t *testing.T) { + dir := t.TempDir() + writeCachedSchema(t, dir, time.Now().AddDate(0, 0, -(staleAfterDays+5))) + + origGlobals := globals + t.Cleanup(func() { globals = origGlobals }) + globals.Config = filepath.Join(dir, "config.yaml") + + apischema.SetConfigDir(dir) + t.Cleanup(func() { apischema.SetConfigDir("") }) + + cmd := &cobra.Command{Use: "api"} + var out, errOut bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&errOut) + + warnIfSchemaStale(cmd) + + if out.Len() != 0 { + t.Errorf("warning leaked to stdout: %q", out.String()) + } + if !strings.Contains(errOut.String(), "mcli schema refresh") { + t.Errorf("stderr = %q, want the refresh hint", errOut.String()) + } +} + +// TestWarnIfSchemaStale_silentWhenFresh verifies a fresh schema produces no noise. +func TestWarnIfSchemaStale_silentWhenFresh(t *testing.T) { + dir := t.TempDir() + writeCachedSchema(t, dir, time.Now()) + + origGlobals := globals + t.Cleanup(func() { globals = origGlobals }) + globals.Config = filepath.Join(dir, "config.yaml") + + apischema.SetConfigDir(dir) + t.Cleanup(func() { apischema.SetConfigDir("") }) + + cmd := &cobra.Command{Use: "api"} + var out, errOut bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&errOut) + + warnIfSchemaStale(cmd) + + if out.Len() != 0 || errOut.Len() != 0 { + t.Errorf("expected no output for a fresh schema, got stdout=%q stderr=%q", out.String(), errOut.String()) + } +} + +// writeCachedSchema writes a parseable schema.graphql into dir and backdates its +// mtime, which is what liveSchemaInfo uses to age a cached schema. +func writeCachedSchema(t *testing.T, dir string, modTime time.Time) { + t.Helper() + path := apischema.CachedSchemaPath(dir) + if err := os.WriteFile(path, []byte("type Query { hello: String }\n"), 0o600); err != nil { + t.Fatalf("write cached schema: %v", err) + } + if err := os.Chtimes(path, modTime, modTime); err != nil { + t.Fatalf("set cache mtime: %v", err) + } +} diff --git a/schema/embed.go b/schema/embed.go index 7870b05..9f99fac 100644 --- a/schema/embed.go +++ b/schema/embed.go @@ -1,7 +1,31 @@ // Package schema exposes the monday.com GraphQL SDL as an embedded string. package schema -import _ "embed" +import ( + _ "embed" + "strings" + "time" +) //go:embed monday.graphql var SDL string + +// fetchedAtRaw is the date the embedded SDL was introspected, in YYYY-MM-DD form. +// It is rewritten by `make schema` (see tools/introspect). +// +//go:embed fetched_at.txt +var fetchedAtRaw string + +// fetchedAtLayout is the date format stored in fetched_at.txt. +const fetchedAtLayout = "2006-01-02" + +// FetchedAt reports the date the embedded SDL was introspected from the monday +// API. It returns the zero Time if the recorded date is missing or unparseable, +// so callers must check IsZero before treating the value as an age. +func FetchedAt() time.Time { + t, err := time.Parse(fetchedAtLayout, strings.TrimSpace(fetchedAtRaw)) + if err != nil { + return time.Time{} + } + return t +} diff --git a/schema/fetched_at.txt b/schema/fetched_at.txt new file mode 100644 index 0000000..14cb8d4 --- /dev/null +++ b/schema/fetched_at.txt @@ -0,0 +1 @@ +2026-06-17 diff --git a/tools/introspect/main.go b/tools/introspect/main.go index e8e776d..978a553 100644 --- a/tools/introspect/main.go +++ b/tools/introspect/main.go @@ -24,9 +24,10 @@ import ( ) const ( - endpoint = "https://api.monday.com/v2" - outFile = "schema/monday.graphql" - httpTimeout = 60 * time.Second + endpoint = "https://api.monday.com/v2" + outFile = "schema/monday.graphql" + fetchedAtFile = "schema/fetched_at.txt" + httpTimeout = 60 * time.Second ) func main() { @@ -62,7 +63,14 @@ func run(ctx context.Context) error { return fmt.Errorf("write %s: %w", outPath, err) } + // Record when this SDL was fetched so the CLI can warn when the embedded + // schema has gone stale (see schema.FetchedAt). + fetchedAt := time.Now().UTC().Format("2006-01-02") + "\n" + if err := os.WriteFile(fetchedAtFile, []byte(fetchedAt), 0o644); err != nil { + return fmt.Errorf("write %s: %w", fetchedAtFile, err) + } + lines := strings.Count(sdl, "\n") - fmt.Fprintf(os.Stderr, "introspect: wrote %s (%d lines)\n", outPath, lines) + fmt.Fprintf(os.Stderr, "introspect: wrote %s (%d lines), %s\n", outPath, lines, fetchedAtFile) return nil } From 000b9d0098ca1c1d89d3584a32dec4d23fe81799 Mon Sep 17 00:00:00 2001 From: Arnon Rotem-Gal-Oz Date: Tue, 8 Sep 2026 13:44:32 +0300 Subject: [PATCH 5/6] feat(board): return column descriptions and share the pretty renderer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A column description is the only place a board records what a column *means*, which matters most to the next agent reading it. 'board column create --description' wrote one, but no read path returned it: the field was missing from every column selection, so 'board column list' and 'board get' dropped it and 'column describe' never echoed the text it had just set. Adds description to all five column selections (the two reads, plus the three mutations so writes echo back), and surfaces it in --pretty. The two column tables now share one renderer instead of each carrying its own tabwriter, and the duplicate boardColumn struct — field-for-field identical to columnOutput — is gone; that duplication is what let the two views drift. DESCRIPTION is the last column so long prose cannot widen the columns to its left. truncate now counts runes rather than bytes: it is fed user-authored descriptions, where byte slicing would split a rune and emit U+FFFD. Co-Authored-By: Claude Opus 5 --- internal/api/gen/generated.go | 32 ++++++++++++++++++++ internal/api/queries/boards.graphql | 5 ++++ internal/cli/api_list.go | 7 +++-- internal/cli/board.go | 23 ++++----------- internal/cli/board_column.go | 46 ++++++++++++++++++++++++----- 5 files changed, 86 insertions(+), 27 deletions(-) diff --git a/internal/api/gen/generated.go b/internal/api/gen/generated.go index 60cf7cc..7c0461b 100644 --- a/internal/api/gen/generated.go +++ b/internal/api/gen/generated.go @@ -72,6 +72,8 @@ type BoardColumnCreateCreate_columnColumn struct { Title string `json:"title"` // The column's type. Type ColumnType `json:"type"` + // The column's description. + Description string `json:"description"` // The column's settings in a string form. Settings_str string `json:"settings_str"` // The column's width. @@ -89,6 +91,9 @@ func (v *BoardColumnCreateCreate_columnColumn) GetTitle() string { return v.Titl // GetType returns BoardColumnCreateCreate_columnColumn.Type, and is useful for accessing the field via an interface. func (v *BoardColumnCreateCreate_columnColumn) GetType() ColumnType { return v.Type } +// GetDescription returns BoardColumnCreateCreate_columnColumn.Description, and is useful for accessing the field via an interface. +func (v *BoardColumnCreateCreate_columnColumn) GetDescription() string { return v.Description } + // GetSettings_str returns BoardColumnCreateCreate_columnColumn.Settings_str, and is useful for accessing the field via an interface. func (v *BoardColumnCreateCreate_columnColumn) GetSettings_str() string { return v.Settings_str } @@ -142,6 +147,8 @@ type BoardColumnDescribeChange_column_metadataColumn struct { Title string `json:"title"` // The column's type. Type ColumnType `json:"type"` + // The column's description. + Description string `json:"description"` // The column's settings in a string form. Settings_str string `json:"settings_str"` // The column's width. @@ -159,6 +166,11 @@ func (v *BoardColumnDescribeChange_column_metadataColumn) GetTitle() string { re // GetType returns BoardColumnDescribeChange_column_metadataColumn.Type, and is useful for accessing the field via an interface. func (v *BoardColumnDescribeChange_column_metadataColumn) GetType() ColumnType { return v.Type } +// GetDescription returns BoardColumnDescribeChange_column_metadataColumn.Description, and is useful for accessing the field via an interface. +func (v *BoardColumnDescribeChange_column_metadataColumn) GetDescription() string { + return v.Description +} + // GetSettings_str returns BoardColumnDescribeChange_column_metadataColumn.Settings_str, and is useful for accessing the field via an interface. func (v *BoardColumnDescribeChange_column_metadataColumn) GetSettings_str() string { return v.Settings_str @@ -208,6 +220,8 @@ type BoardColumnListBoardsBoardColumnsColumn struct { Title string `json:"title"` // The column's type. Type ColumnType `json:"type"` + // The column's description. + Description string `json:"description"` // The column's settings in a string form. Settings_str string `json:"settings_str"` // The column's width. @@ -225,6 +239,9 @@ func (v *BoardColumnListBoardsBoardColumnsColumn) GetTitle() string { return v.T // GetType returns BoardColumnListBoardsBoardColumnsColumn.Type, and is useful for accessing the field via an interface. func (v *BoardColumnListBoardsBoardColumnsColumn) GetType() ColumnType { return v.Type } +// GetDescription returns BoardColumnListBoardsBoardColumnsColumn.Description, and is useful for accessing the field via an interface. +func (v *BoardColumnListBoardsBoardColumnsColumn) GetDescription() string { return v.Description } + // GetSettings_str returns BoardColumnListBoardsBoardColumnsColumn.Settings_str, and is useful for accessing the field via an interface. func (v *BoardColumnListBoardsBoardColumnsColumn) GetSettings_str() string { return v.Settings_str } @@ -251,6 +268,8 @@ type BoardColumnRenameChange_column_titleColumn struct { Title string `json:"title"` // The column's type. Type ColumnType `json:"type"` + // The column's description. + Description string `json:"description"` // The column's settings in a string form. Settings_str string `json:"settings_str"` // The column's width. @@ -268,6 +287,9 @@ func (v *BoardColumnRenameChange_column_titleColumn) GetTitle() string { return // GetType returns BoardColumnRenameChange_column_titleColumn.Type, and is useful for accessing the field via an interface. func (v *BoardColumnRenameChange_column_titleColumn) GetType() ColumnType { return v.Type } +// GetDescription returns BoardColumnRenameChange_column_titleColumn.Description, and is useful for accessing the field via an interface. +func (v *BoardColumnRenameChange_column_titleColumn) GetDescription() string { return v.Description } + // GetSettings_str returns BoardColumnRenameChange_column_titleColumn.Settings_str, and is useful for accessing the field via an interface. func (v *BoardColumnRenameChange_column_titleColumn) GetSettings_str() string { return v.Settings_str } @@ -432,6 +454,8 @@ type BoardGetBoardsBoardColumnsColumn struct { Title string `json:"title"` // The column's type. Type ColumnType `json:"type"` + // The column's description. + Description string `json:"description"` // The column's settings in a string form. Settings_str string `json:"settings_str"` // The column's width. @@ -449,6 +473,9 @@ func (v *BoardGetBoardsBoardColumnsColumn) GetTitle() string { return v.Title } // GetType returns BoardGetBoardsBoardColumnsColumn.Type, and is useful for accessing the field via an interface. func (v *BoardGetBoardsBoardColumnsColumn) GetType() ColumnType { return v.Type } +// GetDescription returns BoardGetBoardsBoardColumnsColumn.Description, and is useful for accessing the field via an interface. +func (v *BoardGetBoardsBoardColumnsColumn) GetDescription() string { return v.Description } + // GetSettings_str returns BoardGetBoardsBoardColumnsColumn.Settings_str, and is useful for accessing the field via an interface. func (v *BoardGetBoardsBoardColumnsColumn) GetSettings_str() string { return v.Settings_str } @@ -21662,6 +21689,7 @@ mutation BoardColumnCreate ($boardId: ID!, $title: String!, $columnType: ColumnT id title type + description settings_str width archived @@ -21748,6 +21776,7 @@ mutation BoardColumnDescribe ($boardId: ID!, $columnId: String!, $description: S id title type + description settings_str width archived @@ -21794,6 +21823,7 @@ query BoardColumnList ($boardId: ID!) { id title type + description settings_str width archived @@ -21835,6 +21865,7 @@ mutation BoardColumnRename ($boardId: ID!, $columnId: String!, $title: String!) id title type + description settings_str width archived @@ -21985,6 +22016,7 @@ query BoardGet ($id: ID!, $withItems: Boolean! = false, $itemsLimit: Int! = 25, id title type + description settings_str width archived diff --git a/internal/api/queries/boards.graphql b/internal/api/queries/boards.graphql index 31dc2e9..67c9791 100644 --- a/internal/api/queries/boards.graphql +++ b/internal/api/queries/boards.graphql @@ -75,6 +75,7 @@ query BoardGet( id title type + description settings_str width archived @@ -181,6 +182,7 @@ query BoardColumnList($boardId: ID!) { id title type + description settings_str width archived @@ -202,6 +204,7 @@ mutation BoardColumnCreate( id title type + description settings_str width archived @@ -222,6 +225,7 @@ mutation BoardColumnRename($boardId: ID!, $columnId: String!, $title: String!) { id title type + description settings_str width archived @@ -234,6 +238,7 @@ mutation BoardColumnDescribe($boardId: ID!, $columnId: String!, $description: St id title type + description settings_str width archived diff --git a/internal/cli/api_list.go b/internal/cli/api_list.go index bf5eed4..c766a3f 100644 --- a/internal/cli/api_list.go +++ b/internal/cli/api_list.go @@ -6,6 +6,7 @@ import ( "sort" "strings" "text/tabwriter" + "unicode/utf8" "github.com/spf13/cobra" @@ -116,8 +117,10 @@ func runAPIList(cmd *cobra.Command, typeFilter string, noBuiltins bool, jsonOut func truncate(s string, max int) string { // Collapse whitespace / newlines in descriptions. s = strings.Join(strings.Fields(s), " ") - if len(s) <= max { + // Count runes, not bytes: column descriptions are user-authored and may be + // non-ASCII, where byte slicing would split a rune and emit a replacement char. + if utf8.RuneCountInString(s) <= max { return s } - return s[:max-1] + "…" + return string([]rune(s)[:max-1]) + "…" } diff --git a/internal/cli/board.go b/internal/cli/board.go index 3038235..154ce13 100644 --- a/internal/cli/board.go +++ b/internal/cli/board.go @@ -190,7 +190,7 @@ type boardGetOutput struct { Workspace *boardWorkspace `json:"workspace,omitempty"` Owners []boardOwner `json:"owners"` Groups []boardGroup `json:"groups"` - Columns []boardColumn `json:"columns"` + Columns []columnOutput `json:"columns"` // Items and ItemsCursor are populated only with --items; both are omitted // otherwise (additive per axiom A6). Items []itemListOutputItem `json:"items,omitempty"` @@ -215,15 +215,6 @@ type boardGroup struct { Position string `json:"position"` } -type boardColumn struct { - ID string `json:"id"` - Title string `json:"title"` - Type string `json:"type"` - SettingsStr string `json:"settings_str"` - Width int `json:"width"` - Archived bool `json:"archived"` -} - func newBoardGetCmd() *cobra.Command { var ( withItems bool @@ -391,7 +382,7 @@ func runBoardGet(cmd *cobra.Command, id string, withItems bool, itemsLimit int, WorkspaceID: b.Workspace_id, Owners: make([]boardOwner, len(b.Owners)), Groups: make([]boardGroup, len(b.Groups)), - Columns: make([]boardColumn, len(b.Columns)), + Columns: make([]columnOutput, len(b.Columns)), } if b.Workspace.Id != "" { @@ -414,10 +405,11 @@ func runBoardGet(cmd *cobra.Command, id string, withItems bool, itemsLimit int, } } for i, c := range b.Columns { - out.Columns[i] = boardColumn{ + out.Columns[i] = columnOutput{ ID: c.Id, Title: c.Title, Type: string(c.Type), + Description: c.Description, SettingsStr: c.Settings_str, Width: c.Width, Archived: c.Archived, @@ -478,12 +470,9 @@ func runBoardGet(cmd *cobra.Command, id string, withItems bool, itemsLimit int, if len(out.Columns) > 0 { _, _ = fmt.Fprintf(o, "\nColumns (%d):\n", len(out.Columns)) - tw := tabwriter.NewWriter(o, 0, 0, 2, ' ', 0) - _, _ = fmt.Fprintln(tw, " ID\tTITLE\tTYPE\tARCHIVED") - for _, c := range out.Columns { - _, _ = fmt.Fprintf(tw, " %s\t%s\t%s\t%v\n", c.ID, c.Title, c.Type, c.Archived) + if err := writeColumnsTable(o, out.Columns, " "); err != nil { + return errs.Internal("flush columns table: %v", err) } - _ = tw.Flush() } if withItems { diff --git a/internal/cli/board_column.go b/internal/cli/board_column.go index 1d04888..d2f5a50 100644 --- a/internal/cli/board_column.go +++ b/internal/cli/board_column.go @@ -3,6 +3,7 @@ package cli import ( "encoding/json" "fmt" + "io" "os" "strings" "text/tabwriter" @@ -91,6 +92,7 @@ type columnOutput struct { ID string `json:"id"` Title string `json:"title"` Type string `json:"type"` + Description string `json:"description,omitempty"` SettingsStr string `json:"settings_str"` Width int `json:"width"` Archived bool `json:"archived"` @@ -147,6 +149,7 @@ func runBoardColumnList(cmd *cobra.Command, boardID string) error { ID: c.Id, Title: c.Title, Type: string(c.Type), + Description: c.Description, SettingsStr: c.Settings_str, Width: c.Width, Archived: c.Archived, @@ -169,10 +172,30 @@ func runBoardColumnList(cmd *cobra.Command, boardID string) error { return err } - w := tabwriter.NewWriter(cmd.OutOrStdout(), 0, 0, 2, ' ', 0) - _, _ = fmt.Fprintln(w, "ID\tTITLE\tTYPE\tARCHIVED") - for _, c := range items { - _, _ = fmt.Fprintf(w, "%s\t%s\t%s\t%v\n", c.ID, c.Title, c.Type, c.Archived) + return writeColumnsTable(cmd.OutOrStdout(), items, "") +} + +// writeColumnDescriptionLine renders a column's description under a single-object +// summary, matching between 'column create' and 'column describe'. Unlike the table +// it is not truncated: there is one column in view and room to read it. +func writeColumnDescriptionLine(o io.Writer, description string) error { + if description == "" { + return nil + } + _, err := fmt.Fprintf(o, " description: %s\n", description) + return err +} + +// writeColumnsTable renders the column table shared by 'board column list' and +// 'board get', so the two views cannot drift apart. DESCRIPTION comes last: it is +// the only free-text field, and trailing it keeps a long description from widening +// every column to its left. indent prefixes each row ('board get' nests its tables). +func writeColumnsTable(o io.Writer, cols []columnOutput, indent string) error { + w := tabwriter.NewWriter(o, 0, 0, 2, ' ', 0) + _, _ = fmt.Fprintf(w, "%sID\tTITLE\tTYPE\tARCHIVED\tDESCRIPTION\n", indent) + for _, c := range cols { + _, _ = fmt.Fprintf(w, "%s%s\t%s\t%s\t%v\t%s\n", + indent, c.ID, c.Title, c.Type, c.Archived, truncate(c.Description, 55)) } return w.Flush() } @@ -249,6 +272,7 @@ func runBoardColumnCreate(cmd *cobra.Command, boardID, title, columnType, descri ID: c.Id, Title: c.Title, Type: string(c.Type), + Description: c.Description, SettingsStr: c.Settings_str, Width: c.Width, Archived: c.Archived, @@ -268,8 +292,10 @@ func runBoardColumnCreate(cmd *cobra.Command, boardID, title, columnType, descri return err } - _, err = fmt.Fprintf(cmd.OutOrStdout(), "Created column %s: %s (%s)\n", out.ID, out.Title, out.Type) - return err + if _, err = fmt.Fprintf(cmd.OutOrStdout(), "Created column %s: %s (%s)\n", out.ID, out.Title, out.Type); err != nil { + return err + } + return writeColumnDescriptionLine(cmd.OutOrStdout(), out.Description) } // columnDeleteOutput is the JSON shape for column delete. @@ -394,6 +420,7 @@ func runBoardColumnRename(cmd *cobra.Command, boardID, columnID, title string) e ID: c.Id, Title: c.Title, Type: string(c.Type), + Description: c.Description, SettingsStr: c.Settings_str, Width: c.Width, Archived: c.Archived, @@ -470,6 +497,7 @@ func runBoardColumnDescribe(cmd *cobra.Command, boardID, columnID, description s ID: c.Id, Title: c.Title, Type: string(c.Type), + Description: c.Description, SettingsStr: c.Settings_str, Width: c.Width, Archived: c.Archived, @@ -489,6 +517,8 @@ func runBoardColumnDescribe(cmd *cobra.Command, boardID, columnID, description s return err } - _, err = fmt.Fprintf(cmd.OutOrStdout(), "Updated description for column %s: %s\n", out.ID, out.Title) - return err + if _, err = fmt.Fprintf(cmd.OutOrStdout(), "Updated description for column %s: %s\n", out.ID, out.Title); err != nil { + return err + } + return writeColumnDescriptionLine(cmd.OutOrStdout(), out.Description) } From 4a434616df374b04009089e9ce1d577b55a7d64c Mon Sep 17 00:00:00 2001 From: Arnon Rotem-Gal-Oz Date: Tue, 8 Sep 2026 13:45:59 +0300 Subject: [PATCH 6/6] docs: rewrite the agent skill doc and guard it against drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skill doc is mcli's primary contract for LLM callers, and nothing tied it to the command tree — which is how it came to omit `board create --workspace` entirely, a flag whose absence produces `API: User unauthorized to perform action`, an error that says nothing about workspaces. Meanwhile the optional Semantic Layer section spent 23% of the doc's budget. The doc is now organised by what an agent actually does, in frequency order: discover IDs, read, write, recover from an error. Board setup, previously first, is the rarest operation. It gains a "Failures Worth Knowing About In Advance" section for the cases the error message alone will not explain, and the error table now says what to do next per code rather than just naming it. It moves out of a Go string literal into skill.md, embedded with //go:embed. Markdown is full of backticks and a Go raw string cannot contain one, so the old `+ "`" +` form made the doc effectively unreviewable — which is precisely how it drifted. TestSkillDoc_MatchesCommandTree resolves every command path and flag the doc mentions (85 and 79 respectively, including inside fenced blocks) against the real cobra tree, and TestSkillDoc_AuditCatchesDrift proves that audit can still fail. TestSkill_Concise now budgets bytes rather than lines, because bytes are what the doc costs an agent's context: reflowing a paragraph changes the line count without changing the cost, while one dense table row costs far more than one short line. Also: README and demo examples refreshed to real output, two new demos, ADR-002 records the never-emitted CONFLICT code's removal, and the stale plan docs are deleted per the repo's convention that committed docs describe shipped reality (git history preserves them). Co-Authored-By: Claude Opus 5 --- .claude/skills/update-docs/SKILL.md | 157 ++++++++++ .gitignore | 8 +- README.md | 165 +++++++++- examples/crm-demo.md | 257 ++++++++++++++++ examples/crm-demo.sh | 286 ++++++++++++++++++ examples/ecommerce-demo.md | 36 ++- examples/ecommerce-demo.sh | 48 +-- examples/portfolio-demo.md | 311 +++++++++++++++++++ examples/portfolio-demo.sh | 306 +++++++++++++++++++ go.mod | 2 +- internal/cli/skill.go | 208 +------------ internal/cli/skill.md | 233 +++++++++++++++ internal/cli/skill_test.go | 250 +++++++++++++++- memory-bank/adrs/ADR-002-cli-grammar.md | 23 +- memory-bank/adrs/ADR-003-llm-skill-api.md | 6 +- memory-bank/archive/plan-api-version.md | 102 ------- memory-bank/archive/plan-v0.md | 202 ------------- memory-bank/docs/plan-daemon-webhooks.md | 165 ---------- memory-bank/docs/plan-dynamic-api.md | 347 ---------------------- memory-bank/docs/plan-phase4.5.md | 86 ------ memory-bank/docs/plan-phase5.md | 179 ----------- 21 files changed, 2050 insertions(+), 1327 deletions(-) create mode 100644 .claude/skills/update-docs/SKILL.md create mode 100644 examples/crm-demo.md create mode 100644 examples/crm-demo.sh create mode 100644 examples/portfolio-demo.md create mode 100644 examples/portfolio-demo.sh create mode 100644 internal/cli/skill.md delete mode 100644 memory-bank/archive/plan-api-version.md delete mode 100644 memory-bank/archive/plan-v0.md delete mode 100644 memory-bank/docs/plan-daemon-webhooks.md delete mode 100644 memory-bank/docs/plan-dynamic-api.md delete mode 100644 memory-bank/docs/plan-phase4.5.md delete mode 100644 memory-bank/docs/plan-phase5.md diff --git a/.claude/skills/update-docs/SKILL.md b/.claude/skills/update-docs/SKILL.md new file mode 100644 index 0000000..529efdf --- /dev/null +++ b/.claude/skills/update-docs/SKILL.md @@ -0,0 +1,157 @@ +--- +name: update-docs +description: "Update mcli's documentation to match the code: the generated skill doc, README, examples, ADRs, and memory-bank plans. Use after any change that adds, renames, or removes a command, flag, error code, or output shape." +allowed-tools: default +--- + +# Skill: UPDATE-DOCS (mcli) + +Bring mcli's documentation back in line with the code. Invoke after a change that +is user-visible, or standalone with `/update-docs`. + +mcli is consumed primarily by LLM agents, not humans reading a wiki. A stale doc +is not cosmetic debt here — it actively makes agents emit wrong commands. Treat +documentation drift as a bug. + +## The doc surfaces (in priority order) + +### 1. `internal/cli/skill.md` — the skill doc (highest priority) + +Plain markdown, embedded into the binary by `internal/cli/skill.go`. It is what +`mcli skill` / `mcli describe` prints, and the primary contract for LLM callers. +It MUST be updated when a change touches: + +- a command or subcommand (added, renamed, removed) +- a flag that appears in the doc's usage lines +- an error code or its exit code (see `internal/errs/errs.go`) +- an output shape, output mode, or column-value write shape + +Organise it by what an agent does, in frequency order — discover IDs, read, write, +recover from an error — not by the shape of the command tree. Board setup is rare; +writing items is not. Document the flags whose *absence* produces a misleading error +(`board create --workspace`, `board column create --defaults`) even when `--help` +calls them optional, and keep the "Failures Worth Knowing About In Advance" section +current: it is the highest-value section in the doc. + +Guards that will fail CI if you forget: + +- `TestSkillDoc_MatchesCommandTree` — every command path and flag the doc shows is + resolved against the real cobra tree. This is the guard that did not exist when + the doc drifted; `TestSkillDoc_AuditCatchesDrift` proves it can still fail. +- `TestSkill_ErrorCodesMatchErrs` — every code in `errs.AllCodes()` must appear in + the doc annotated with the exit code `errs.ToExitCode` really returns. +- `TestSkill_ContainsGoals` / `TestSkill_ContainsSections` — required commands and + section headings. +- `TestSkill_Concise` — a byte budget (`skillDocMaxBytes`), because bytes are what + the doc costs an agent's context. If you are near it, cut or tighten an existing + section rather than raising the cap. + +Run `go test ./internal/cli/ -run TestSkill` after editing. + +### 2. `README.md` + +Human-facing entry point. Update the command reference and any example output +when commands change. Keep it consistent with the skill doc — they should never +disagree about a flag name. + +### 3. `examples/` + +JSON request/response examples (`item-create.json`, `board-get.json`, …) and the +end-to-end demos (`ecommerce-demo.md` / `.sh`). If an output shape or column-value +encoding changed, these become wrong and misleading. Verify affected examples still +reflect real output. + +### 4. `memory-bank/adrs/` — validate, do not rewrite + +ADR-001 (stack), ADR-002 (CLI grammar / exit codes), ADR-003 (LLM skill API), +ADR-004 (secret storage). ADRs record decisions as of a date; they are **not** +auto-updated. If a change contradicts an ADR, flag it for human review — either the +change is wrong or the ADR needs a superseding entry. Never silently edit an ADR to +match new code. + +### 5. `memory-bank/` plans and docs + +See the hygiene rules below — this repo has a specific convention. + +## Plan hygiene rules (mcli-specific) + +The committed part of `memory-bank/` describes **shipped reality**. It is not a +roadmap and not a scratchpad. Concretely: + +- **Committed docs must not contain speculative or unstarted plans.** Aspirational + plan docs rot into lies: readers cannot tell "not_started" from "done but never + re-marked", which is exactly the failure this repo already hit. +- **Future / in-design work goes in `memory-bank/private/plans/`**, which is + gitignored (`memory-bank/private/`). Design docs awaiting approval live there. +- **When a plan fully ships, delete it** rather than leaving a COMPLETED doc behind. + Git history preserves it, and the code plus ADRs are the durable record. +- **Never trust a `Status:` marker.** Verify against the codebase before acting on + or reporting any plan's status. + +`memory-bank/axioms.md` and `memory-bank/projectbrief.md` are durable — update them +only when a project-level invariant or goal genuinely changes. + +## Out of scope for this repo + +- `schema/monday.graphql` is vendored, not documentation. Refresh it with + `make schema` (runs `tools/introspect` against the live API), not by hand. +- `.claude/` is gitignored except for this skill; do not add docs there expecting + them to be committed. + +## Core workflow + +### Step 1: Establish the diff + +```bash +git diff HEAD --name-only # default: uncommitted +git diff HEAD --stat +``` + +With `--since-push`: + +```bash +git diff origin/$(git branch --show-current) --name-only +``` + +### Step 2: Map code changes to doc surfaces + +For each changed file, ask which surfaces above it invalidates. The common cases: + +| Changed | Update | +|---|---| +| `internal/cli/*.go` (command/flag) | skill doc, README, examples | +| `internal/errs/errs.go` | skill doc error codes (test enforces) | +| output/formatting code | skill doc output shapes, examples, README | +| `internal/api/**` behaviour | examples, possibly ADR-003 validation | + +### Step 3: Apply or propose + +With edit permission, make the changes and run the guard tests. Without it, present +a diff and wait for approval. + +### Step 4: Report + +```markdown +## Documentation Update Summary + +### Applied +- ✅ `internal/cli/skill.go`: added `mcli item batch`, updated error-code line +- ✅ `README.md`: command reference row for `item batch` +- ✅ `examples/item-create.json`: refreshed to current output shape + +### Verified clean +- `memory-bank/axioms.md` — no invariant changed + +### Flagged for human review +- ⚠️ Change alters exit code for API errors — contradicts ADR-002, needs a + superseding ADR or a revert + +### Tests +- `go test ./internal/cli/ -run TestSkill` — pass +``` + +## Exit conditions + +- Every invalidated surface is updated or explicitly reported as needing review +- Guard tests pass (`go test ./internal/cli/ -run TestSkill`) +- No committed doc describes unshipped work diff --git a/.gitignore b/.gitignore index af25bc8..a686333 100644 --- a/.gitignore +++ b/.gitignore @@ -23,7 +23,11 @@ vendor/ # Private memory-bank area (per setup-memory-bank convention) memory-bank/private/ -# Symlinks to external tooling (not part of this repo) -.claude/ +# Symlinks to external tooling (not part of this repo). +# Exception: .claude/skills/update-docs is a real, repo-specific skill we ship. +.claude/* +!.claude/skills/ +.claude/skills/* +!.claude/skills/update-docs/ CLAUDE.md AGENTS.md diff --git a/README.md b/README.md index 57d0b2d..9b5524d 100644 --- a/README.md +++ b/README.md @@ -35,11 +35,19 @@ mcli item list --board 9832181507 --subitems # Get a single item, including its subitems with column values mcli item get 1234567890 --subitems -# Create an item with column values +# Create an item with typed column shorthands +mcli item create --board 9832181507 --name "Ship feature" \ + --status "Working on it" --due 2026-06-01 + +# Or with raw column JSON, for any column type mcli item create --board 9832181507 --name "Ship feature" \ --col 'status={"label":"Working on it"}' \ --col 'due_date={"date":"2026-06-01"}' +# Create or update many items in one command (rate-limit friendly) +echo '[{"name":"Ship v1","status":"Done"},{"name":"Write docs","due":"2026-06-10"}]' \ + | mcli item create --board 9832181507 - + # Run a raw GraphQL query mcli query 'query { me { id name } }' ``` @@ -88,6 +96,9 @@ mcli board group list/create/rename/archive/delete mcli board column list/create/rename/describe/delete mcli item list/get/create/update/move/archive/delete +mcli item create/update ... --status/--due/--date/--number/--text/--checkbox + Typed column shorthands +mcli item create/update --board - Batch write, rows on stdin mcli item find --board --column --value Find by column value mcli item post-update --body Post a comment mcli item get-updates [--limit N] Read comments/updates @@ -100,6 +111,9 @@ mcli api list [--type query|mutation] Browse all ~250 API operations from mcli api describe Inspect signature and argument types mcli api [--arg k=v]... Execute any operation; JSON args auto-coerced +mcli schema status Report which schema is in use and how old it is +mcli schema refresh Fetch the live schema with your token and cache it + mcli query '' Raw GraphQL queries (inline, -f file, -f -) mcli query save/list/run/delete Saved query management @@ -178,6 +192,24 @@ mcli config set routing-key # add baggage: routingKey= he mcli config set routing-key "" # clear routing key ``` +### Keeping the schema fresh + +The embedded schema is a snapshot taken when the binary was built, so a long-lived +install gradually falls behind the live API. `mcli schema refresh` introspects the API +with your own token and caches the result in `~/.config/mcli/schema.graphql`, which takes +precedence over the embedded copy — no rebuild, no release wait. + +```sh +mcli schema status # source, api-version, age, type count +mcli schema refresh # fetch + cache; prints the type delta +mcli schema refresh --api-version 2026-08 # one-off fetch, not persisted to config +``` + +`mcli api` warns on stderr (never stdout) when the schema in use is more than 30 days +old. Refreshing is always explicit: no mcli command fetches a schema behind your back. +To drop the cache and go back to the embedded schema, delete the cached file or run +`mcli config set api-version default`. + ## Output Modes Control output format with a global flag or persist a default: @@ -211,7 +243,7 @@ mcli daemon start --url https://your-server.example.com ``` The daemon: -- Listens for webhook payloads on an HTTP port (default 6780) +- Listens for webhook payloads on an HTTP port (default 8420) - Exposes a Unix socket IPC for CLI commands - Opens a Cloudflare Quick Tunnel for a public URL (requires `cloudflared` in PATH) - Re-registers webhooks automatically when the tunnel URL changes @@ -266,20 +298,113 @@ This outputs a concise goal-oriented guide that tells the LLM what commands exis On **read**, column values are decoded to human-readable form (status labels, ISO dates, etc.). -On **write**, pass monday's raw column-value JSON via `--col =`: +On **write** there are two paths: typed shorthands for the common types, and raw +`--col =` for everything else. + +### Typed shorthands + +| Flag | Column type | Accepts | +|------|-------------|---------| +| `--status