Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .surface
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ hey boxes
hey boxes --all
hey boxes --limit
hey calendars
hey collection
hey collections
hey collections --all
hey collections --limit
hey commands
hey completion
hey compose
Expand Down
105 changes: 105 additions & 0 deletions internal/cmd/collection.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package cmd

import (
"fmt"
"strconv"

"github.com/spf13/cobra"

"github.com/basecamp/hey-cli/internal/htmlutil"
"github.com/basecamp/hey-cli/internal/output"
)

// maxCollectionPages bounds the timeline walk in case pagination never
// terminates. A collection's threads plus interleaved calendar events span at
// most a few dozen pages in practice; this is a safety backstop.
const maxCollectionPages = 200

type collectionCommand struct {
cmd *cobra.Command
}

func newCollectionCommand() *collectionCommand {
collectionCommand := &collectionCommand{}
collectionCommand.cmd = &cobra.Command{
Use: "collection <id>",
Short: "List topics in a collection",
Annotations: map[string]string{
"agent_notes": "Lists all threads (topics) in a collection, walking the full timeline. Use topic IDs with hey threads to read them. Get collection IDs from hey collections.",
},
Example: ` hey collection 8328
hey collection 8328 --json`,
RunE: collectionCommand.run,
Args: usageExactOneArg(),
}

return collectionCommand
}

func (c *collectionCommand) run(cmd *cobra.Command, args []string) error {
if err := requireAuth(); err != nil {
return err
}

collectionID, err := strconv.ParseInt(args[0], 10, 64)
if err != nil {
return output.ErrUsage(fmt.Sprintf("invalid collection ID: %s", args[0]))
}

// The collection detail endpoint is HTML-only (no JSON representation) and
// renders only the first ~50 threads per page. The rest live further down
// the timeline, reachable via the "next page" pagination link, so walk every
// page and accumulate the threads, de-duplicating across pages.
ctx := cmd.Context()
path := fmt.Sprintf("/collections/%d", collectionID)
var topics []htmlutil.CollectionTopic

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Empty collections return "data": null in JSON/quiet output instead of an empty list, forcing clients to handle a different result type. Initialize topics to an empty slice.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/cmd/collection.go, line 55:

<comment>Empty collections return `"data": null` in JSON/quiet output instead of an empty list, forcing clients to handle a different result type. Initialize `topics` to an empty slice.</comment>

<file context>
@@ -0,0 +1,105 @@
+	// page and accumulate the threads, de-duplicating across pages.
+	ctx := cmd.Context()
+	path := fmt.Sprintf("/collections/%d", collectionID)
+	var topics []htmlutil.CollectionTopic
+	seen := map[int64]bool{}
+	var truncated bool
</file context>
Suggested change
var topics []htmlutil.CollectionTopic
topics := []htmlutil.CollectionTopic{}

seen := map[int64]bool{}
var truncated bool

for page := 0; path != ""; page++ {
if page >= maxCollectionPages {
truncated = true
break
}
resp, err := sdk.GetHTML(ctx, path)
if err != nil {
return convertSDKError(err)
}
body := string(resp.Data)
for _, t := range htmlutil.ParseCollectionTopicsHTML(body) {
if !seen[t.TopicID] {
seen[t.TopicID] = true
topics = append(topics, t)
}
}
path = htmlutil.ParseCollectionNextPage(body)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: A collection page containing an absolute https:// next link makes the next authenticated request go to that host, exposing the OAuth bearer token. Restrict parsed pagination links to a relative collection path (or validate same-origin after resolving it) before assigning path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/cmd/collection.go, line 75:

<comment>A collection page containing an absolute `https://` next link makes the next authenticated request go to that host, exposing the OAuth bearer token. Restrict parsed pagination links to a relative collection path (or validate same-origin after resolving it) before assigning `path`.</comment>

<file context>
@@ -0,0 +1,105 @@
+				topics = append(topics, t)
+			}
+		}
+		path = htmlutil.ParseCollectionNextPage(body)
+	}
+
</file context>

}

var notice string
if truncated {
notice = fmt.Sprintf("Stopped after %d pages; some older threads may be missing.", maxCollectionPages)
}

if writer.IsStyled() {
table := newTable(cmd.OutOrStdout())
table.addRow([]string{"Topic ID", "Title"})
for _, t := range topics {
table.addRow([]string{fmt.Sprintf("%d", t.TopicID), t.Title})
}
table.print()
if notice != "" {
fmt.Fprintln(cmd.OutOrStdout(), notice)
}
return nil
}

return writeOK(topics,
output.WithSummary(fmt.Sprintf("%d topics in collection %d", len(topics), collectionID)),
output.WithNotice(notice),
output.WithBreadcrumbs(output.Breadcrumb{
Action: "read",
Command: "hey threads <topic-id>",
Description: "Read a thread in this collection",
}),
)
}
89 changes: 89 additions & 0 deletions internal/cmd/collections.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package cmd

import (
"fmt"
"sort"

"github.com/spf13/cobra"

"github.com/basecamp/hey-sdk/go/pkg/generated"

"github.com/basecamp/hey-cli/internal/output"
)

type collectionsCommand struct {
cmd *cobra.Command
limit int
all bool
}

func newCollectionsCommand() *collectionsCommand {
collectionsCommand := &collectionsCommand{}
collectionsCommand.cmd = &cobra.Command{
Use: "collections",
Short: "List collections",
Annotations: map[string]string{
"agent_notes": "Returns all email collections (labels). Sorted by name. Use --json for full detail.",
},
Example: ` hey collections
hey collections --limit 5
hey collections --json`,
RunE: collectionsCommand.run,
}

collectionsCommand.cmd.Flags().IntVar(&collectionsCommand.limit, "limit", 0, "Maximum number of collections to show")
collectionsCommand.cmd.Flags().BoolVar(&collectionsCommand.all, "all", false, "Fetch all results (override --limit)")

return collectionsCommand
}

func (c *collectionsCommand) run(cmd *cobra.Command, args []string) error {
if err := requireAuth(); err != nil {
return err
}

ctx := cmd.Context()

// Collections have no dedicated SDK service; fetch via the raw JSON endpoint.
resp, err := sdk.Get(ctx, "/collections.json")
if err != nil {
return convertSDKError(err)
}

var collections []generated.Collection
if err := resp.UnmarshalData(&collections); err != nil {
return fmt.Errorf("decode collections: %w", err)
}

sort.Slice(collections, func(i, j int) bool {
return collections[i].Name < collections[j].Name
})

total := len(collections)
if c.limit > 0 && !c.all && len(collections) > c.limit {
collections = collections[:c.limit]
}
notice := output.TruncationNotice(len(collections), total)

if writer.IsStyled() {
table := newTable(cmd.OutOrStdout())
table.addRow([]string{"ID", "Name", "Updated"})
for _, col := range collections {
table.addRow([]string{
fmt.Sprintf("%d", col.Id),
col.Name,
col.UpdatedAt.Format("2006-01-02"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Calling col.UpdatedAt.Format("2006-01-02") directly will render as 0001-01-01 when the timestamp is zero-valued. Other commands in this codebase (e.g., box.go) use the formatDate helper to display an empty string instead. Using formatDate here would maintain consistency and avoid confusing output.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/cmd/collections.go, line 75:

<comment>Calling `col.UpdatedAt.Format("2006-01-02")` directly will render as `0001-01-01` when the timestamp is zero-valued. Other commands in this codebase (e.g., `box.go`) use the `formatDate` helper to display an empty string instead. Using `formatDate` here would maintain consistency and avoid confusing output.</comment>

<file context>
@@ -0,0 +1,89 @@
+			table.addRow([]string{
+				fmt.Sprintf("%d", col.Id),
+				col.Name,
+				col.UpdatedAt.Format("2006-01-02"),
+			})
+		}
</file context>
Suggested change
col.UpdatedAt.Format("2006-01-02"),
formatDate(col.UpdatedAt),

})
Comment on lines +72 to +76
}
table.print()
if notice != "" {
fmt.Fprintln(cmd.OutOrStdout(), notice)
}
return nil
}

return writeOK(collections,
output.WithSummary(fmt.Sprintf("%d collections", len(collections))),
output.WithNotice(notice),
)
}
2 changes: 2 additions & 0 deletions internal/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ func newRootCmd() *cobra.Command {
root.AddCommand(newAuthCommand().cmd)
root.AddCommand(newBoxesCommand().cmd)
root.AddCommand(newBoxCommand().cmd)
root.AddCommand(newCollectionsCommand().cmd)
root.AddCommand(newCollectionCommand().cmd)
root.AddCommand(newThreadsCommand().cmd)
root.AddCommand(newReplyCommand().cmd)
root.AddCommand(newComposeCommand().cmd)
Expand Down
72 changes: 72 additions & 0 deletions internal/htmlutil/collections.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package htmlutil

import (
"html"
"regexp"
"strconv"
"strings"
)

var (
// collectionAnchorRe matches an <a> anchor linking to a topic, capturing the
// attributes before the href, the topic id, the attributes after the href,
// and the inner HTML. Attribute order varies, so class/title are inspected
// from the combined attribute text rather than assumed adjacent to href.
collectionAnchorRe = regexp.MustCompile(`(?s)<a\b([^>]*?)href="/topics/(\d+)[^"]*"([^>]*)>(.*?)</a>`)
titleAttrRe = regexp.MustCompile(`title="([^"]*)"`)
anyTagRe = regexp.MustCompile(`<[^>]+>`)
// collectionNextPageRe finds the timeline's "next page" pagination link.
collectionNextPageRe = regexp.MustCompile(`data-pagination-target="nextPageLink"[^>]*href="([^"]+)"`)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Collections silently truncate when the pagination anchor emits href before data-pagination-target, because this regex treats attribute order as significant. Parse the anchor attributes independently (or handle both orders) so the full timeline walk remains robust.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/htmlutil/collections.go, line 19:

<comment>Collections silently truncate when the pagination anchor emits `href` before `data-pagination-target`, because this regex treats attribute order as significant. Parse the anchor attributes independently (or handle both orders) so the full timeline walk remains robust.</comment>

<file context>
@@ -0,0 +1,72 @@
+	titleAttrRe        = regexp.MustCompile(`title="([^"]*)"`)
+	anyTagRe           = regexp.MustCompile(`<[^>]+>`)
+	// collectionNextPageRe finds the timeline's "next page" pagination link.
+	collectionNextPageRe = regexp.MustCompile(`data-pagination-target="nextPageLink"[^>]*href="([^"]+)"`)
+)
+
</file context>

)
Comment on lines +18 to +20

// CollectionTopic is a single topic (thread) listed within a collection.
type CollectionTopic struct {
TopicID int64 `json:"topic_id"`
Title string `json:"title"`
}

// ParseCollectionTopicsHTML extracts the threads on one page of a collection.
// Threads render either as card__link anchors (the recent items shown as cards)
// or entry__collection-topic anchors (older items further down the timeline).
// Other /topics/ links on the page — such as attachment "show message" links —
// are ignored. Results preserve document order and are de-duplicated by topic ID
// within the page. The collection detail endpoint has no JSON representation, so
// this HTML-based extraction is required.
func ParseCollectionTopicsHTML(page string) []CollectionTopic {
var topics []CollectionTopic
seen := map[int64]bool{}
for _, m := range collectionAnchorRe.FindAllStringSubmatch(page, -1) {
attrs := m[1] + m[3]
if !strings.Contains(attrs, "card__link") && !strings.Contains(attrs, "entry__collection-topic") {
continue
}
id, err := strconv.ParseInt(m[2], 10, 64)
if err != nil || seen[id] {
continue
}
seen[id] = true

// Prefer the title attribute (a clean subject); fall back to inner text.
var title string
if tm := titleAttrRe.FindStringSubmatch(attrs); tm != nil {
title = tm[1]
} else {
title = anyTagRe.ReplaceAllString(m[4], "")
}
title = strings.TrimSpace(html.UnescapeString(title))

topics = append(topics, CollectionTopic{TopicID: id, Title: title})
}
return topics
}

// ParseCollectionNextPage returns the path of the next page of a collection's
// timeline, or "" when there is no further page. The path is used to walk the
// whole collection, since a single page only renders the first ~50 threads.
func ParseCollectionNextPage(page string) string {
m := collectionNextPageRe.FindStringSubmatch(page)
if m == nil {
return ""
}
return html.UnescapeString(m[1])
}
43 changes: 43 additions & 0 deletions internal/htmlutil/collections_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package htmlutil

import "testing"

func TestParseCollectionTopicsHTML(t *testing.T) {
page := `
<article class="card card--topic">
<a class="card__link" href="/topics/111">Flight confirmation</a>
</article>
<article class="card card--topic">
<a class="card__link" href="/topics/111">Flight confirmation</a>
</article>
<div class="entry">
<a title="Orsay Museum &amp; tickets" class="entry__collection-topic undecorated" data-action="x" href="/topics/222">Re: Orsay Museum &amp; tickets</a>
</div>
<div class="attachment">
<a aria-label="Show message" class="undecorated" href="/topics/111#__entry_9">receipt.pdf</a>
</div>`

got := ParseCollectionTopicsHTML(page)
if len(got) != 2 {
t.Fatalf("expected 2 topics, got %d: %+v", len(got), got)
}
if got[0].TopicID != 111 || got[0].Title != "Flight confirmation" {
t.Errorf("topic[0] = %+v, want {111, Flight confirmation}", got[0])
}
// Title should come from the title attribute (unescaped), not the "Re:" inner text.
if got[1].TopicID != 222 || got[1].Title != "Orsay Museum & tickets" {
t.Errorf("topic[1] = %+v, want {222, Orsay Museum & tickets}", got[1])
}
}

func TestParseCollectionNextPage(t *testing.T) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The existing test only exercises the case where data-pagination-target precedes href. Adding a test case with href appearing before data-pagination-target would guard against the attribute-order sensitivity of the regex and serve as a regression test for correct pagination extraction.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/htmlutil/collections_test.go, line 33:

<comment>The existing test only exercises the case where `data-pagination-target` precedes `href`. Adding a test case with `href` appearing before `data-pagination-target` would guard against the attribute-order sensitivity of the regex and serve as a regression test for correct pagination extraction.</comment>

<file context>
@@ -0,0 +1,43 @@
+	}
+}
+
+func TestParseCollectionNextPage(t *testing.T) {
+	with := `<a class="pagination-link" data-pagination-target="nextPageLink" href="/collections/5?page=abc%3D%3D&amp;x=1">next</a>`
+	if got := ParseCollectionNextPage(with); got != "/collections/5?page=abc%3D%3D&x=1" {
</file context>

with := `<a class="pagination-link" data-pagination-target="nextPageLink" href="/collections/5?page=abc%3D%3D&amp;x=1">next</a>`
if got := ParseCollectionNextPage(with); got != "/collections/5?page=abc%3D%3D&x=1" {
t.Errorf("ParseCollectionNextPage = %q, want unescaped path", got)
}
Comment on lines +34 to +37

without := `<div>no pagination here</div>`
if got := ParseCollectionNextPage(without); got != "" {
t.Errorf("ParseCollectionNextPage = %q, want empty", got)
}
}
Loading