From fa40e6210ae6efa5b00e3256f31b3f521b7cc89d Mon Sep 17 00:00:00 2001 From: Caleb Piekstra Date: Tue, 3 Nov 2026 07:05:49 -0500 Subject: [PATCH 1/2] feat(pins): add pins add, remove and list slck can now pin and unpin a message and list what is pinned in a channel, through pins.add, pins.remove and pins.list. Pinning an already pinned message, or unpinning one that is not pinned, succeeds and says so. A missing_scope error names the scope the command needs (pins:write or pins:read). The extended manifest gains pins:write. --- README.md | 36 ++- docs/development.md | 4 +- integration-tests.md | 16 +- internal/client/errors.go | 12 + internal/client/errors_test.go | 15 ++ internal/client/pins.go | 63 +++++ internal/cmd/pins/add.go | 59 +++++ internal/cmd/pins/list.go | 107 ++++++++ internal/cmd/pins/pins.go | 38 +++ internal/cmd/pins/pins_test.go | 239 ++++++++++++++++++ internal/cmd/pins/remove.go | 59 +++++ .../cmd/pins/testdata/already-pinned.json | 1 + .../cmd/pins/testdata/channel-not-found.json | 1 + internal/cmd/pins/testdata/missing-scope.json | 1 + internal/cmd/pins/testdata/no-pin.json | 1 + internal/cmd/pins/testdata/not-pinned.json | 1 + internal/cmd/pins/testdata/ok.json | 1 + .../cmd/pins/testdata/pins-list-empty.json | 1 + internal/cmd/pins/testdata/pins-list.json | 44 ++++ .../pins/testdata/users-info-U0000000001.json | 1 + .../pins/testdata/users-info-U0000000002.json | 1 + internal/cmd/root/root.go | 2 + 22 files changed, 699 insertions(+), 4 deletions(-) create mode 100644 internal/client/pins.go create mode 100644 internal/cmd/pins/add.go create mode 100644 internal/cmd/pins/list.go create mode 100644 internal/cmd/pins/pins.go create mode 100644 internal/cmd/pins/pins_test.go create mode 100644 internal/cmd/pins/remove.go create mode 100644 internal/cmd/pins/testdata/already-pinned.json create mode 100644 internal/cmd/pins/testdata/channel-not-found.json create mode 100644 internal/cmd/pins/testdata/missing-scope.json create mode 100644 internal/cmd/pins/testdata/no-pin.json create mode 100644 internal/cmd/pins/testdata/not-pinned.json create mode 100644 internal/cmd/pins/testdata/ok.json create mode 100644 internal/cmd/pins/testdata/pins-list-empty.json create mode 100644 internal/cmd/pins/testdata/pins-list.json create mode 100644 internal/cmd/pins/testdata/users-info-U0000000001.json create mode 100644 internal/cmd/pins/testdata/users-info-U0000000002.json diff --git a/README.md b/README.md index 1c7c199..acdd4dc 100644 --- a/README.md +++ b/README.md @@ -304,6 +304,7 @@ or `op read ... | slck set-credential --key bot_token --stdin`. "mpim:read", "mpim:write", "pins:read", + "pins:write", "reactions:read", "reactions:write", "reminders:read", @@ -345,6 +346,7 @@ or `op read ... | slck set-credential --key bot_token --stdin`. "mpim:read", "mpim:write", "pins:read", + "pins:write", "reactions:read", "reactions:write", "reminders:read", @@ -452,7 +454,8 @@ The **extended manifest** (see the collapsible section above) adds these capabil | `usergroups:read` | Resolve `@subteam` / user-group mentions | | `app_mentions:read` | Receive @-mention events (needed for event subscriptions) | | `reactions:read` | List reactions on a message | -| Other `*:read` scopes (`bookmarks`, `pins`, `reminders`, `links`, `lists`, `calls`, `dnd`, `metadata.message`, `remote_files`) | Read-only access to those domains; mostly forward-compat for commands that may use them | +| `pins:read` / `pins:write` | List, add and remove pinned messages: `slck pins list/add/remove` | +| Other `*:read` scopes (`bookmarks`, `reminders`, `links`, `lists`, `calls`, `dnd`, `metadata.message`, `remote_files`) | Read-only access to those domains; mostly forward-compat for commands that may use them | ### Token Types @@ -706,6 +709,36 @@ slck messages unreact C1234567890 1234567890.123456 thumbsup reactions: :+1: 2 (alice, bob), :eyes: 1 (carol) ``` +### Pins + +```bash +# Pin a message (the timestamp can also be p-prefixed or the message's Slack URL) +slck pins add C1234567890 1234567890.123456 + +# List what is pinned in a channel, most recently pinned first +slck pins list C1234567890 + +# Unpin a message +slck pins remove C1234567890 1234567890.123456 +``` + +`add` and `remove` need the `pins:write` scope and `list` needs `pins:read`, on the bot token, or on the user token with `--as-user`. Neither scope is in the default manifest; the extended manifest has both. Adding a pin that exists, or removing one that does not, succeeds and says so. + +`list` prints one row per pin; `remove` takes the `TS` value. A pinned file has `-` for `TS` and `AUTHOR`. + +```text +TS | PINNED | PINNED_BY | AUTHOR | TEXT +1699999000.000200 | 2023-11-14 22:16 | bob | alice | On call this week: @bob +``` + +#### Pins Command Reference + +| Command | Flags | Description | +|---------|-------|-------------| +| `add ` | | Pin a message (idempotent) | +| `remove ` | | Unpin a message (idempotent) | +| `list ` | | List pinned items | + ### Search > **Note:** Search requires a user token (`xoxp-*`). See [Token Types](#token-types). @@ -1022,6 +1055,7 @@ Commands have convenient aliases: | `channels` | `ch` | | `users` | `u` | | `messages` | `msg`, `m` | +| `pins` | `pin` | | `search` | `s` | | `emoji` | `e` | | `workspace` | `ws`, `team` | diff --git a/docs/development.md b/docs/development.md index 47f3591..bdac299 100644 --- a/docs/development.md +++ b/docs/development.md @@ -6,8 +6,8 @@ Collective standards and automation remain canonical in their own repositories. ## Project Overview slack-chat-api builds the `slck` command-line interface for Slack. It supports -channel management, user lookup, messaging, reactions, message history, and -workspace information. +channel management, user lookup, messaging, reactions, pins, message history, +and workspace information. ## Quick Commands diff --git a/integration-tests.md b/integration-tests.md index adbdc1d..dc0887f 100644 --- a/integration-tests.md +++ b/integration-tests.md @@ -36,6 +36,8 @@ In **OAuth & Permissions**, add these **Bot Token Scopes**: | `team:read` | Get workspace info | Part 2 | | `chat:write` | Send, update, delete messages | Part 3 | | `reactions:write` | Add/remove reactions | Part 3 | +| `pins:read` | List pinned messages | Part 3 | +| `pins:write` | Pin and unpin messages | Part 3 | | `channels:manage` | Create, archive, set topic/purpose, invite | Parts 4 & 5 | | `groups:write` | Topic/purpose/invite for private channels | Part 4 | | `emoji:read` | List custom workspace emoji | Part 7 | @@ -164,7 +166,7 @@ These tests don't modify anything. Safe to run anytime. ## Part 3: Messaging Tests -**Scopes required:** `chat:write`, `reactions:write` +**Scopes required:** `chat:write`, `reactions:write`, `pins:read`, `pins:write` These tests create messages, then clean them up at the end. @@ -197,6 +199,18 @@ Using **TS₁** from step 3.1: | 5 | `slck messages unreact $TEST_CHANNEL_ID thumbsup` | "Removed :thumbsup: reaction" | | 6 | `slck messages unreact $TEST_CHANNEL_ID heart` | "Removed :heart: reaction" | +### 3.3a Pins + +Using **TS₁** from step 3.1: + +| Step | Command | Expected | +|------|---------|----------| +| 1 | `slck pins add $TEST_CHANNEL_ID ` | "Pinned message in $TEST_CHANNEL_ID" | +| 2 | `slck pins add $TEST_CHANNEL_ID ` | "Message is already pinned in $TEST_CHANNEL_ID" (idempotent, exit 0) | +| 3 | `slck pins list $TEST_CHANNEL_ID` | `TS \| PINNED \| PINNED_BY \| AUTHOR \| TEXT` header, then a row whose TS is TS₁ | +| 4 | `slck pins remove $TEST_CHANNEL_ID ` | "Unpinned message in $TEST_CHANNEL_ID" | +| 5 | `slck pins remove $TEST_CHANNEL_ID ` | "Message is not pinned in $TEST_CHANNEL_ID" (idempotent, exit 0) | + ### 3.4 Threading Using **TS₁** from step 3.1: diff --git a/internal/client/errors.go b/internal/client/errors.go index 80342f8..2451cd2 100644 --- a/internal/client/errors.go +++ b/internal/client/errors.go @@ -57,3 +57,15 @@ func WrapError(operation string, err error) error { return fmt.Errorf("%s: %w", operation, err) } + +// WrapScopeError is WrapError for a call that needs one known OAuth scope: +// a missing_scope error names that scope and where to grant it. +func WrapScopeError(operation, scope string, err error) error { + if !IsSlackError(err, "missing_scope") { + return WrapError(operation, err) + } + hint := fmt.Sprintf("The token is missing the %s scope. Add %s to the app's Bot Token Scopes "+ + "(or User Token Scopes for --as-user) at api.slack.com/apps, then reinstall the app to the workspace.", + scope, scope) + return fmt.Errorf("%s: %w\nHint: %s", operation, err, hint) +} diff --git a/internal/client/errors_test.go b/internal/client/errors_test.go index 033a8aa..b736684 100644 --- a/internal/client/errors_test.go +++ b/internal/client/errors_test.go @@ -190,3 +190,18 @@ func TestWrapError_OperationContext(t *testing.T) { assert.Contains(t, errStr, "archive channel C123") assert.Contains(t, errStr, "already_archived") } + +func TestWrapScopeError(t *testing.T) { + assert.NoError(t, WrapScopeError("pin message", "pins:write", nil)) + + err := WrapScopeError("pin message", "pins:write", fmt.Errorf("slack API error: missing_scope")) + require.Error(t, err) + assert.Equal(t, "pin message: slack API error: missing_scope\n"+ + "Hint: The token is missing the pins:write scope. Add pins:write to the app's Bot Token Scopes "+ + "(or User Token Scopes for --as-user) at api.slack.com/apps, then reinstall the app to the workspace.", err.Error()) + + err = WrapScopeError("pin message", "pins:write", fmt.Errorf("slack API error: channel_not_found")) + require.Error(t, err) + assert.Contains(t, err.Error(), "Verify the channel ID is correct") + assert.NotContains(t, err.Error(), "pins:write") +} diff --git a/internal/client/pins.go b/internal/client/pins.go new file mode 100644 index 0000000..6d6a65b --- /dev/null +++ b/internal/client/pins.go @@ -0,0 +1,63 @@ +package client + +import ( + "encoding/json" + "net/url" +) + +// PinnedItem is one entry of pins.list. Type is "message" for a pinned +// message; Slack can also return "file" and "file_comment" items, which carry +// File instead of Message. +type PinnedItem struct { + Type string `json:"type"` + Channel string `json:"channel"` + Created int64 `json:"created"` + CreatedBy string `json:"created_by"` + Message *Message `json:"message,omitempty"` + File *File `json:"file,omitempty"` +} + +// AddPin pins a message to a channel. It reports whether the message was +// already pinned, which Slack signals with an already_pinned error. +func (c *Client) AddPin(channel, timestamp string) (alreadyPinned bool, err error) { + _, err = c.post("pins.add", map[string]interface{}{ + "channel": channel, + "timestamp": timestamp, + }) + if IsSlackError(err, "already_pinned") { + return true, nil + } + return false, err +} + +// RemovePin unpins a message from a channel. It reports whether the message +// was not pinned, which Slack signals with a no_pin or not_pinned error. +func (c *Client) RemovePin(channel, timestamp string) (notPinned bool, err error) { + _, err = c.post("pins.remove", map[string]interface{}{ + "channel": channel, + "timestamp": timestamp, + }) + if IsSlackError(err, "no_pin") || IsSlackError(err, "not_pinned") { + return true, nil + } + return false, err +} + +// ListPins returns the items pinned to a channel. +func (c *Client) ListPins(channel string) ([]PinnedItem, error) { + params := url.Values{} + params.Set("channel", channel) + + body, err := c.get("pins.list", params) + if err != nil { + return nil, err + } + + var result struct { + Items []PinnedItem `json:"items"` + } + if err := json.Unmarshal(body, &result); err != nil { + return nil, err + } + return result.Items, nil +} diff --git a/internal/cmd/pins/add.go b/internal/cmd/pins/add.go new file mode 100644 index 0000000..769a886 --- /dev/null +++ b/internal/cmd/pins/add.go @@ -0,0 +1,59 @@ +package pins + +import ( + "github.com/spf13/cobra" + + "github.com/open-cli-collective/slack-chat-api/internal/client" + "github.com/open-cli-collective/slack-chat-api/internal/output" + "github.com/open-cli-collective/slack-chat-api/internal/validate" +) + +func newAddCmd() *cobra.Command { + return &cobra.Command{ + Use: "add ", + Short: "Pin a message to a channel", + Long: `Pin a message to a channel. + +The timestamp is the message's ts (1234567890.123456), its p-prefixed form, +or the message's Slack URL. Pinning a message that is already pinned +succeeds and says so. Requires the pins:write scope.`, + Example: ` slck pins add C1234567890 1234567890.123456 + slck pins add general 1234567890.123456 --as-user`, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + quiet(cmd) + return runAdd(args[0], args[1], nil) + }, + } +} + +func runAdd(channel, timestamp string, c *client.Client) error { + if err := validate.Timestamp(timestamp); err != nil { + return err + } + timestamp = validate.NormalizeTimestamp(timestamp) + + if c == nil { + var err error + c, err = client.New() + if err != nil { + return err + } + } + + channelID, err := c.ResolveMessageDestination(channel) + if err != nil { + return err + } + + alreadyPinned, err := c.AddPin(channelID, timestamp) + if err != nil { + return client.WrapScopeError("pin message", writeScope, err) + } + if alreadyPinned { + output.Printf("Message %s is already pinned in %s\n", timestamp, channel) + return nil + } + output.Printf("Pinned message %s in %s\n", timestamp, channel) + return nil +} diff --git a/internal/cmd/pins/list.go b/internal/cmd/pins/list.go new file mode 100644 index 0000000..c590a5c --- /dev/null +++ b/internal/cmd/pins/list.go @@ -0,0 +1,107 @@ +package pins + +import ( + "sort" + "strconv" + + "github.com/spf13/cobra" + + "github.com/open-cli-collective/slack-chat-api/internal/client" + "github.com/open-cli-collective/slack-chat-api/internal/output" + "github.com/open-cli-collective/slack-chat-api/internal/render" +) + +// pinTextMaxRunes is the width of the TEXT column, the one-line message width +// messages history uses. +const pinTextMaxRunes = 80 + +func newListCmd() *cobra.Command { + return &cobra.Command{ + Use: "list ", + Short: "List the items pinned to a channel", + Long: `List the items pinned to a channel, most recently pinned first. + +Each row has the message's TS (what pins remove takes), when it was pinned, +who pinned it, its author and its text, cut to 80 characters. Requires the +pins:read scope.`, + Example: ` slck pins list C1234567890 + slck pins list general --as-user`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + quiet(cmd) + return runList(args[0], nil) + }, + } +} + +func runList(channel string, c *client.Client) error { + if c == nil { + var err error + c, err = client.New() + if err != nil { + return err + } + } + + channelID, err := c.ResolveMessageDestination(channel) + if err != nil { + return err + } + + items, err := c.ListPins(channelID) + if err != nil { + return client.WrapScopeError("list pins", readScope, err) + } + + if len(items) == 0 { + output.Printf("No pins in %s\n", channel) + return nil + } + + sort.SliceStable(items, func(i, j int) bool { return items[i].Created > items[j].Created }) + + resolver := client.NewUserResolver(c) + headers := []string{"TS", "PINNED", "PINNED_BY", "AUTHOR", "TEXT"} + rows := make([][]string, 0, len(items)) + for _, item := range items { + rows = append(rows, pinRow(item, resolver)) + } + output.SearchTable(headers, rows, pinTextMaxRunes) + return nil +} + +// pinRow renders one pinned item as a table row. A file pin has no message, +// so its TS and AUTHOR are "-" and its TEXT names the file. +func pinRow(item client.PinnedItem, resolver *client.UserResolver) []string { + pinned := "-" + if item.Created > 0 { + pinned = render.Timestamp(strconv.FormatInt(item.Created, 10)) + } + pinnedBy := orDash(resolver.Resolve(item.CreatedBy)) + + if item.Message != nil { + m := *item.Message + body, _ := render.MessageBody(m, resolver) + return []string{orDash(m.TS), pinned, pinnedBy, render.MessageAuthor(m, resolver), orDash(body)} + } + + text := "[" + item.Type + "]" + if f := item.File; f != nil { + name := f.Title + if name == "" { + name = f.Name + } + if name == "" { + name = f.ID + } + text += " " + name + } + return []string{"-", pinned, pinnedBy, "-", text} +} + +func orDash(s string) string { + if s == "" { + return "-" + } + return s +} diff --git a/internal/cmd/pins/pins.go b/internal/cmd/pins/pins.go new file mode 100644 index 0000000..47d60f4 --- /dev/null +++ b/internal/cmd/pins/pins.go @@ -0,0 +1,38 @@ +package pins + +import ( + "github.com/spf13/cobra" +) + +// The OAuth scopes the pins methods need, on a bot or a user token alike. +const ( + readScope = "pins:read" + writeScope = "pins:write" +) + +// NewCmd creates the pins command with all subcommands +func NewCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "pins", + Aliases: []string{"pin"}, + Short: "Manage pinned messages", + Long: `Pin and unpin messages in a channel, and list what is pinned. + +add and remove need the pins:write scope and list needs pins:read, on the bot +token, or on the user token with --as-user.`, + } + + cmd.AddCommand(newAddCmd()) + cmd.AddCommand(newRemoveCmd()) + cmd.AddCommand(newListCmd()) + + return cmd +} + +// quiet stops cobra from printing the usage text and its own copy of a +// runtime error once the arguments parsed, so the root command prints the +// error and its hint alone. +func quiet(cmd *cobra.Command) { + cmd.SilenceUsage = true + cmd.SilenceErrors = true +} diff --git a/internal/cmd/pins/pins_test.go b/internal/cmd/pins/pins_test.go new file mode 100644 index 0000000..af68fe7 --- /dev/null +++ b/internal/cmd/pins/pins_test.go @@ -0,0 +1,239 @@ +package pins + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/open-cli-collective/slack-chat-api/internal/client" + "github.com/open-cli-collective/slack-chat-api/internal/output" + "github.com/open-cli-collective/slack-chat-api/internal/render" +) + +const ( + testChannel = "C1234567890" + testTS = "1234567890.123456" +) + +// slackCall is one request the fake Slack API received. +type slackCall struct { + method string + path string + query map[string]string + body map[string]interface{} +} + +// fakeSlack answers each API method with a testdata fixture and records the +// requests. users.info answers with users-info-.json. +func fakeSlack(t *testing.T, fixtures map[string]string) (*client.Client, *[]slackCall) { + t.Helper() + var calls []slackCall + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + call := slackCall{method: r.Method, path: r.URL.Path, query: map[string]string{}} + for k := range r.URL.Query() { + call.query[k] = r.URL.Query().Get(k) + } + if r.Method == http.MethodPost { + assert.NoError(t, json.NewDecoder(r.Body).Decode(&call.body)) + } + calls = append(calls, call) + + fixture, ok := fixtures[r.URL.Path] + if r.URL.Path == "/users.info" { + fixture, ok = "users-info-"+call.query["user"]+".json", true + } + if !assert.True(t, ok, "unexpected call to %s", r.URL.Path) { + http.Error(w, "unexpected call", http.StatusInternalServerError) + return + } + b, err := os.ReadFile(filepath.Join("testdata", fixture)) + if !assert.NoError(t, err) { + http.Error(w, "missing fixture", http.StatusInternalServerError) + return + } + _, _ = w.Write(b) + })) + t.Cleanup(server.Close) + return client.NewWithConfig(server.URL, "test-token", nil), &calls +} + +func captureOutput(t *testing.T, fn func()) string { + t.Helper() + var buf strings.Builder + orig := output.Writer + output.Writer = &buf + t.Cleanup(func() { output.Writer = orig }) + fn() + return buf.String() +} + +func TestRunAdd(t *testing.T) { + for name, tc := range map[string]struct { + fixture string + want string + }{ + "pins": {"ok.json", "Pinned message 1234567890.123456 in C1234567890\n"}, + "already pinned": {"already-pinned.json", "Message 1234567890.123456 is already pinned in C1234567890\n"}, + } { + t.Run(name, func(t *testing.T) { + c, calls := fakeSlack(t, map[string]string{"/pins.add": tc.fixture}) + + out := captureOutput(t, func() { + require.NoError(t, runAdd(testChannel, testTS, c)) + }) + + assert.Equal(t, tc.want, out) + require.Len(t, *calls, 1) + call := (*calls)[0] + assert.Equal(t, http.MethodPost, call.method) + assert.Equal(t, "/pins.add", call.path) + assert.Equal(t, map[string]interface{}{"channel": testChannel, "timestamp": testTS}, call.body) + }) + } +} + +func TestRunAdd_NormalizesTimestamp(t *testing.T) { + for _, input := range []string{ + "p1234567890123456", + "https://example.slack.com/archives/C1234567890/p1234567890123456", + } { + t.Run(input, func(t *testing.T) { + c, calls := fakeSlack(t, map[string]string{"/pins.add": "ok.json"}) + + captureOutput(t, func() { + require.NoError(t, runAdd(testChannel, input, c)) + }) + + require.Len(t, *calls, 1) + assert.Equal(t, testTS, (*calls)[0].body["timestamp"]) + }) + } +} + +func TestRunAdd_InvalidTimestamp(t *testing.T) { + c, calls := fakeSlack(t, nil) + + err := runAdd(testChannel, "not-a-timestamp", c) + + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid timestamp") + assert.Empty(t, *calls) +} + +func TestRunRemove(t *testing.T) { + for name, tc := range map[string]struct { + fixture string + want string + }{ + "unpins": {"ok.json", "Unpinned message 1234567890.123456 in C1234567890\n"}, + "not pinned, no_pin": {"no-pin.json", "Message 1234567890.123456 is not pinned in C1234567890\n"}, + "not pinned, not_pinned": {"not-pinned.json", "Message 1234567890.123456 is not pinned in C1234567890\n"}, + } { + t.Run(name, func(t *testing.T) { + c, calls := fakeSlack(t, map[string]string{"/pins.remove": tc.fixture}) + + out := captureOutput(t, func() { + require.NoError(t, runRemove(testChannel, testTS, c)) + }) + + assert.Equal(t, tc.want, out) + require.Len(t, *calls, 1) + call := (*calls)[0] + assert.Equal(t, http.MethodPost, call.method) + assert.Equal(t, "/pins.remove", call.path) + assert.Equal(t, map[string]interface{}{"channel": testChannel, "timestamp": testTS}, call.body) + }) + } +} + +func TestRunRemove_InvalidTimestamp(t *testing.T) { + c, calls := fakeSlack(t, nil) + + err := runRemove(testChannel, "not-a-timestamp", c) + + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid timestamp") + assert.Empty(t, *calls) +} + +func TestRunList(t *testing.T) { + c, calls := fakeSlack(t, map[string]string{"/pins.list": "pins-list.json"}) + + out := captureOutput(t, func() { + require.NoError(t, runList(testChannel, c)) + }) + + want := "TS | PINNED | PINNED_BY | AUTHOR | TEXT\n" + + "1699999000.000200 | " + render.Timestamp("1700000200") + " | bob | alice | On call this week: @bob\n" + + "- | " + render.Timestamp("1700000100") + " | bob | - | [file] Runbook\n" + + "1699990000.000100 | " + render.Timestamp("1700000000") + " | alice | bob | Deploy checklist Step one ¦ step two\n" + assert.Equal(t, want, out) + require.NotEmpty(t, *calls) + assert.Equal(t, http.MethodGet, (*calls)[0].method) + assert.Equal(t, "/pins.list", (*calls)[0].path) + assert.Equal(t, testChannel, (*calls)[0].query["channel"]) +} + +func TestRunList_Empty(t *testing.T) { + c, _ := fakeSlack(t, map[string]string{"/pins.list": "pins-list-empty.json"}) + + out := captureOutput(t, func() { + require.NoError(t, runList(testChannel, c)) + }) + + assert.Equal(t, "No pins in C1234567890\n", out) +} + +func TestMissingScopeNamesTheScope(t *testing.T) { + for name, tc := range map[string]struct { + path string + run func(*client.Client) error + scope string + }{ + "add": {"/pins.add", func(c *client.Client) error { return runAdd(testChannel, testTS, c) }, "pins:write"}, + "remove": {"/pins.remove", func(c *client.Client) error { return runRemove(testChannel, testTS, c) }, "pins:write"}, + "list": {"/pins.list", func(c *client.Client) error { return runList(testChannel, c) }, "pins:read"}, + } { + t.Run(name, func(t *testing.T) { + c, _ := fakeSlack(t, map[string]string{tc.path: "missing-scope.json"}) + + err := tc.run(c) + + require.Error(t, err) + assert.Contains(t, err.Error(), "missing_scope") + assert.Contains(t, err.Error(), "The token is missing the "+tc.scope+" scope. Add "+tc.scope+" to the app's") + }) + } +} + +func TestOtherErrorsKeepTheirHint(t *testing.T) { + c, _ := fakeSlack(t, map[string]string{"/pins.add": "channel-not-found.json"}) + + err := runAdd(testChannel, testTS, c) + + require.Error(t, err) + assert.Contains(t, err.Error(), "pin message: slack API error: channel_not_found") + assert.Contains(t, err.Error(), "Verify the channel ID is correct") + assert.NotContains(t, err.Error(), "pins:write") +} + +func TestRuntimeErrorsPrintNoUsage(t *testing.T) { + cmd := NewCmd() + var out strings.Builder + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SetArgs([]string{"add", testChannel, "not-a-timestamp"}) + + err := cmd.Execute() + + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid timestamp") + assert.Empty(t, out.String()) +} diff --git a/internal/cmd/pins/remove.go b/internal/cmd/pins/remove.go new file mode 100644 index 0000000..a5ba9d6 --- /dev/null +++ b/internal/cmd/pins/remove.go @@ -0,0 +1,59 @@ +package pins + +import ( + "github.com/spf13/cobra" + + "github.com/open-cli-collective/slack-chat-api/internal/client" + "github.com/open-cli-collective/slack-chat-api/internal/output" + "github.com/open-cli-collective/slack-chat-api/internal/validate" +) + +func newRemoveCmd() *cobra.Command { + return &cobra.Command{ + Use: "remove ", + Short: "Unpin a message from a channel", + Long: `Unpin a message from a channel. The message itself is not changed. + +The timestamp takes the same forms as pins add; pins list prints it in the +TS column. Unpinning a message that is not pinned succeeds and says so. +Requires the pins:write scope.`, + Example: ` slck pins remove C1234567890 1234567890.123456 + slck pins remove general 1234567890.123456 --as-user`, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + quiet(cmd) + return runRemove(args[0], args[1], nil) + }, + } +} + +func runRemove(channel, timestamp string, c *client.Client) error { + if err := validate.Timestamp(timestamp); err != nil { + return err + } + timestamp = validate.NormalizeTimestamp(timestamp) + + if c == nil { + var err error + c, err = client.New() + if err != nil { + return err + } + } + + channelID, err := c.ResolveMessageDestination(channel) + if err != nil { + return err + } + + notPinned, err := c.RemovePin(channelID, timestamp) + if err != nil { + return client.WrapScopeError("unpin message", writeScope, err) + } + if notPinned { + output.Printf("Message %s is not pinned in %s\n", timestamp, channel) + return nil + } + output.Printf("Unpinned message %s in %s\n", timestamp, channel) + return nil +} diff --git a/internal/cmd/pins/testdata/already-pinned.json b/internal/cmd/pins/testdata/already-pinned.json new file mode 100644 index 0000000..60272d1 --- /dev/null +++ b/internal/cmd/pins/testdata/already-pinned.json @@ -0,0 +1 @@ +{"ok":false,"error":"already_pinned"} diff --git a/internal/cmd/pins/testdata/channel-not-found.json b/internal/cmd/pins/testdata/channel-not-found.json new file mode 100644 index 0000000..1afd2a1 --- /dev/null +++ b/internal/cmd/pins/testdata/channel-not-found.json @@ -0,0 +1 @@ +{"ok":false,"error":"channel_not_found"} diff --git a/internal/cmd/pins/testdata/missing-scope.json b/internal/cmd/pins/testdata/missing-scope.json new file mode 100644 index 0000000..b712c75 --- /dev/null +++ b/internal/cmd/pins/testdata/missing-scope.json @@ -0,0 +1 @@ +{"ok":false,"error":"missing_scope"} diff --git a/internal/cmd/pins/testdata/no-pin.json b/internal/cmd/pins/testdata/no-pin.json new file mode 100644 index 0000000..267a95c --- /dev/null +++ b/internal/cmd/pins/testdata/no-pin.json @@ -0,0 +1 @@ +{"ok":false,"error":"no_pin"} diff --git a/internal/cmd/pins/testdata/not-pinned.json b/internal/cmd/pins/testdata/not-pinned.json new file mode 100644 index 0000000..4bece8d --- /dev/null +++ b/internal/cmd/pins/testdata/not-pinned.json @@ -0,0 +1 @@ +{"ok":false,"error":"not_pinned"} diff --git a/internal/cmd/pins/testdata/ok.json b/internal/cmd/pins/testdata/ok.json new file mode 100644 index 0000000..31bc455 --- /dev/null +++ b/internal/cmd/pins/testdata/ok.json @@ -0,0 +1 @@ +{"ok":true} diff --git a/internal/cmd/pins/testdata/pins-list-empty.json b/internal/cmd/pins/testdata/pins-list-empty.json new file mode 100644 index 0000000..95c734e --- /dev/null +++ b/internal/cmd/pins/testdata/pins-list-empty.json @@ -0,0 +1 @@ +{"ok":true,"items":[]} diff --git a/internal/cmd/pins/testdata/pins-list.json b/internal/cmd/pins/testdata/pins-list.json new file mode 100644 index 0000000..91f7193 --- /dev/null +++ b/internal/cmd/pins/testdata/pins-list.json @@ -0,0 +1,44 @@ +{ + "ok": true, + "items": [ + { + "type": "message", + "channel": "C1234567890", + "created": 1700000000, + "created_by": "U0000000001", + "message": { + "type": "message", + "user": "U0000000002", + "text": "Deploy checklist\nStep one | step two", + "ts": "1699990000.000100", + "permalink": "https://example.slack.com/archives/C1234567890/p1699990000000100", + "pinned_to": ["C1234567890"] + } + }, + { + "type": "file", + "channel": "C1234567890", + "created": 1700000100, + "created_by": "U0000000002", + "file": { + "id": "F0000000001", + "name": "runbook.pdf", + "title": "Runbook" + } + }, + { + "type": "message", + "channel": "C1234567890", + "created": 1700000200, + "created_by": "U0000000002", + "message": { + "type": "message", + "user": "U0000000001", + "text": "On call this week: <@U0000000002>", + "ts": "1699999000.000200", + "permalink": "https://example.slack.com/archives/C1234567890/p1699999000000200", + "pinned_to": ["C1234567890"] + } + } + ] +} diff --git a/internal/cmd/pins/testdata/users-info-U0000000001.json b/internal/cmd/pins/testdata/users-info-U0000000001.json new file mode 100644 index 0000000..42da1aa --- /dev/null +++ b/internal/cmd/pins/testdata/users-info-U0000000001.json @@ -0,0 +1 @@ +{"ok":true,"user":{"id":"U0000000001","name":"alice","real_name":"Alice","profile":{"display_name":"alice"}}} diff --git a/internal/cmd/pins/testdata/users-info-U0000000002.json b/internal/cmd/pins/testdata/users-info-U0000000002.json new file mode 100644 index 0000000..2bba37a --- /dev/null +++ b/internal/cmd/pins/testdata/users-info-U0000000002.json @@ -0,0 +1 @@ +{"ok":true,"user":{"id":"U0000000002","name":"bob","real_name":"Bob","profile":{"display_name":"bob"}}} diff --git a/internal/cmd/root/root.go b/internal/cmd/root/root.go index 7453153..c13c66d 100644 --- a/internal/cmd/root/root.go +++ b/internal/cmd/root/root.go @@ -20,6 +20,7 @@ import ( "github.com/open-cli-collective/slack-chat-api/internal/cmd/initcmd" "github.com/open-cli-collective/slack-chat-api/internal/cmd/me" "github.com/open-cli-collective/slack-chat-api/internal/cmd/messages" + "github.com/open-cli-collective/slack-chat-api/internal/cmd/pins" "github.com/open-cli-collective/slack-chat-api/internal/cmd/search" "github.com/open-cli-collective/slack-chat-api/internal/cmd/setcred" "github.com/open-cli-collective/slack-chat-api/internal/cmd/unreads" @@ -165,6 +166,7 @@ func init() { rootCmd.AddCommand(users.NewCmd()) rootCmd.AddCommand(unreads.NewCmd()) rootCmd.AddCommand(messages.NewCmd()) + rootCmd.AddCommand(pins.NewCmd()) rootCmd.AddCommand(search.NewCmd()) rootCmd.AddCommand(workspace.NewCmd()) rootCmd.AddCommand(me.NewCmd()) From 114e9ead154385a0f0e1d3e1b07217f328ae94cf Mon Sep 17 00:00:00 2001 From: Caleb Piekstra Date: Tue, 3 Nov 2026 07:12:42 -0500 Subject: [PATCH 2/2] refactor(cmd): share the quiet helper between apps and pins apps and pins each had their own copy of the helper that stops cobra printing a runtime error twice with the usage text. It now lives in internal/cmd/cmdutil, and docs/development.md tells new commands to use it. --- docs/development.md | 2 ++ internal/cmd/apps/apps.go | 21 +++++++-------------- internal/cmd/cmdutil/cmdutil.go | 12 ++++++++++++ internal/cmd/pins/add.go | 3 ++- internal/cmd/pins/list.go | 3 ++- internal/cmd/pins/pins.go | 8 -------- internal/cmd/pins/remove.go | 3 ++- 7 files changed, 27 insertions(+), 25 deletions(-) create mode 100644 internal/cmd/cmdutil/cmdutil.go diff --git a/docs/development.md b/docs/development.md index bdac299..32e8688 100644 --- a/docs/development.md +++ b/docs/development.md @@ -44,6 +44,8 @@ slack-chat-api/ - New commands live under the appropriate `internal/cmd//` package. - Register new commands from the resource root command. - Add tests around the command runner with a mock or injectable client. +- Call `cmdutil.Quiet(cmd)` first in `RunE`, so a runtime error prints once, + with its hint and without the usage text. Argument errors still print usage. ## Output Contract diff --git a/internal/cmd/apps/apps.go b/internal/cmd/apps/apps.go index 6d9452f..8548397 100644 --- a/internal/cmd/apps/apps.go +++ b/internal/cmd/apps/apps.go @@ -16,6 +16,7 @@ import ( "golang.org/x/term" "github.com/open-cli-collective/slack-chat-api/internal/client" + "github.com/open-cli-collective/slack-chat-api/internal/cmd/cmdutil" "github.com/open-cli-collective/slack-chat-api/internal/output" ) @@ -66,14 +67,6 @@ func wrap(operation string, err error) error { return fmt.Errorf("%s: %w", operation, err) } -// quiet stops cobra from printing a runtime error and the usage text: once -// the arguments parsed, the root command prints the error alone, and a -// silent exitError (diff's "differs") prints nothing more. -func quiet(cmd *cobra.Command) { - cmd.SilenceUsage = true - cmd.SilenceErrors = true -} - // NewCmd builds `slck apps`. func NewCmd() *cobra.Command { cmd := &cobra.Command{ @@ -111,7 +104,7 @@ it is printed as JSON whatever --output says.`, slck apps get A0123456789 --to-file app-manifest.json`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - quiet(cmd) + cmdutil.Quiet(cmd) return runGet(newSession(), args[0], toFile) }, } @@ -155,7 +148,7 @@ and non-zero, listing each problem, when it is not.`, slck apps validate --file app-manifest.yaml --app A0123456789`, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - quiet(cmd) + cmdutil.Quiet(cmd) return runValidate(newSession(), file, appID) }, } @@ -193,7 +186,7 @@ could not be made.`, return cannotCompare(cobra.ExactArgs(1)(cmd, args)) }, RunE: func(cmd *cobra.Command, args []string) error { - quiet(cmd) + cmdutil.Quiet(cmd) return runDiff(newSession(), args[0], file) }, } @@ -244,7 +237,7 @@ Basic Information page of the app still shows them.`, Example: ` slck apps create --file app-manifest.json --credentials-file app-credentials.json`, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - quiet(cmd) + cmdutil.Quiet(cmd) return runCreate(newSession(), file, credentialsFile) }, } @@ -312,7 +305,7 @@ is reinstalled to the workspace, and the command says so.`, slck apps update A0123456789 --file app-manifest.json`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - quiet(cmd) + cmdutil.Quiet(cmd) return runUpdate(newSession(), args[0], file) }, } @@ -358,7 +351,7 @@ when stdin is not a terminal.`, Example: ` slck apps delete A0123456789`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - quiet(cmd) + cmdutil.Quiet(cmd) opts.stdin = os.Stdin opts.terminal = term.IsTerminal(int(os.Stdin.Fd())) return runDelete(newSession(), args[0], opts) diff --git a/internal/cmd/cmdutil/cmdutil.go b/internal/cmd/cmdutil/cmdutil.go new file mode 100644 index 0000000..9a2b0a7 --- /dev/null +++ b/internal/cmd/cmdutil/cmdutil.go @@ -0,0 +1,12 @@ +// Package cmdutil holds cobra helpers shared by the command packages. +package cmdutil + +import "github.com/spf13/cobra" + +// Quiet stops cobra from printing a runtime error and the usage text once the +// arguments parsed. The root command then prints the error alone, or nothing +// for an error that reports itself as silent. Call it first in RunE. +func Quiet(cmd *cobra.Command) { + cmd.SilenceUsage = true + cmd.SilenceErrors = true +} diff --git a/internal/cmd/pins/add.go b/internal/cmd/pins/add.go index 769a886..2c14add 100644 --- a/internal/cmd/pins/add.go +++ b/internal/cmd/pins/add.go @@ -4,6 +4,7 @@ import ( "github.com/spf13/cobra" "github.com/open-cli-collective/slack-chat-api/internal/client" + "github.com/open-cli-collective/slack-chat-api/internal/cmd/cmdutil" "github.com/open-cli-collective/slack-chat-api/internal/output" "github.com/open-cli-collective/slack-chat-api/internal/validate" ) @@ -21,7 +22,7 @@ succeeds and says so. Requires the pins:write scope.`, slck pins add general 1234567890.123456 --as-user`, Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - quiet(cmd) + cmdutil.Quiet(cmd) return runAdd(args[0], args[1], nil) }, } diff --git a/internal/cmd/pins/list.go b/internal/cmd/pins/list.go index c590a5c..8b1fe4b 100644 --- a/internal/cmd/pins/list.go +++ b/internal/cmd/pins/list.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" "github.com/open-cli-collective/slack-chat-api/internal/client" + "github.com/open-cli-collective/slack-chat-api/internal/cmd/cmdutil" "github.com/open-cli-collective/slack-chat-api/internal/output" "github.com/open-cli-collective/slack-chat-api/internal/render" ) @@ -28,7 +29,7 @@ pins:read scope.`, slck pins list general --as-user`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - quiet(cmd) + cmdutil.Quiet(cmd) return runList(args[0], nil) }, } diff --git a/internal/cmd/pins/pins.go b/internal/cmd/pins/pins.go index 47d60f4..cb75166 100644 --- a/internal/cmd/pins/pins.go +++ b/internal/cmd/pins/pins.go @@ -28,11 +28,3 @@ token, or on the user token with --as-user.`, return cmd } - -// quiet stops cobra from printing the usage text and its own copy of a -// runtime error once the arguments parsed, so the root command prints the -// error and its hint alone. -func quiet(cmd *cobra.Command) { - cmd.SilenceUsage = true - cmd.SilenceErrors = true -} diff --git a/internal/cmd/pins/remove.go b/internal/cmd/pins/remove.go index a5ba9d6..747d574 100644 --- a/internal/cmd/pins/remove.go +++ b/internal/cmd/pins/remove.go @@ -4,6 +4,7 @@ import ( "github.com/spf13/cobra" "github.com/open-cli-collective/slack-chat-api/internal/client" + "github.com/open-cli-collective/slack-chat-api/internal/cmd/cmdutil" "github.com/open-cli-collective/slack-chat-api/internal/output" "github.com/open-cli-collective/slack-chat-api/internal/validate" ) @@ -21,7 +22,7 @@ Requires the pins:write scope.`, slck pins remove general 1234567890.123456 --as-user`, Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - quiet(cmd) + cmdutil.Quiet(cmd) return runRemove(args[0], args[1], nil) }, }