Skip to content

Register --in/--project flags on comments commands #249

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jeremy merged 3 commits into from
Mar 11, 2026
Merged

Register --in/--project flags on comments commands #249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
14a6b6a
Register --in/--project flags on comments and comment commands
jeremy Mar 11, 2026
74a32d1
Add pingable fallback to ResolvePerson (#250)
jeremy Mar 11, 2026
3ae5aec
Degrade gracefully when pingable fallback fails
jeremy Mar 11, 2026
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
9 changes: 9 additions & 0 deletions .surface
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1853,6 +1853,7 @@ FLAG basecamp comment --count type=bool
FLAG basecamp comment --edit type=bool
FLAG basecamp comment --hints type=bool
FLAG basecamp comment --ids-only type=bool
FLAG basecamp comment --in type=string
FLAG basecamp comment --json type=bool
FLAG basecamp comment --markdown type=bool
FLAG basecamp comment --md type=bool
Expand All @@ -1871,6 +1872,7 @@ FLAG basecamp comments --cache-dir type=string
FLAG basecamp comments --count type=bool
FLAG basecamp comments --hints type=bool
FLAG basecamp comments --ids-only type=bool
FLAG basecamp comments --in type=string
FLAG basecamp comments --json type=bool
FLAG basecamp comments --markdown type=bool
FLAG basecamp comments --md type=bool
Expand All @@ -1889,6 +1891,7 @@ FLAG basecamp comments archive --cache-dir type=string
FLAG basecamp comments archive --count type=bool
FLAG basecamp comments archive --hints type=bool
FLAG basecamp comments archive --ids-only type=bool
FLAG basecamp comments archive --in type=string
FLAG basecamp comments archive --json type=bool
FLAG basecamp comments archive --markdown type=bool
FLAG basecamp comments archive --md type=bool
Expand All @@ -1908,6 +1911,7 @@ FLAG basecamp comments create --count type=bool
FLAG basecamp comments create --edit type=bool
FLAG basecamp comments create --hints type=bool
FLAG basecamp comments create --ids-only type=bool
FLAG basecamp comments create --in type=string
FLAG basecamp comments create --json type=bool
FLAG basecamp comments create --markdown type=bool
FLAG basecamp comments create --md type=bool
Expand All @@ -1927,6 +1931,7 @@ FLAG basecamp comments list --cache-dir type=string
FLAG basecamp comments list --count type=bool
FLAG basecamp comments list --hints type=bool
FLAG basecamp comments list --ids-only type=bool
FLAG basecamp comments list --in type=string
FLAG basecamp comments list --json type=bool
FLAG basecamp comments list --limit type=int
FLAG basecamp comments list --markdown type=bool
Expand All @@ -1947,6 +1952,7 @@ FLAG basecamp comments restore --cache-dir type=string
FLAG basecamp comments restore --count type=bool
FLAG basecamp comments restore --hints type=bool
FLAG basecamp comments restore --ids-only type=bool
FLAG basecamp comments restore --in type=string
FLAG basecamp comments restore --json type=bool
FLAG basecamp comments restore --markdown type=bool
FLAG basecamp comments restore --md type=bool
Expand All @@ -1965,6 +1971,7 @@ FLAG basecamp comments show --cache-dir type=string
FLAG basecamp comments show --count type=bool
FLAG basecamp comments show --hints type=bool
FLAG basecamp comments show --ids-only type=bool
FLAG basecamp comments show --in type=string
FLAG basecamp comments show --json type=bool
FLAG basecamp comments show --markdown type=bool
FLAG basecamp comments show --md type=bool
Expand All @@ -1983,6 +1990,7 @@ FLAG basecamp comments trash --cache-dir type=string
FLAG basecamp comments trash --count type=bool
FLAG basecamp comments trash --hints type=bool
FLAG basecamp comments trash --ids-only type=bool
FLAG basecamp comments trash --in type=string
FLAG basecamp comments trash --json type=bool
FLAG basecamp comments trash --markdown type=bool
FLAG basecamp comments trash --md type=bool
Expand All @@ -2001,6 +2009,7 @@ FLAG basecamp comments update --cache-dir type=string
FLAG basecamp comments update --count type=bool
FLAG basecamp comments update --hints type=bool
FLAG basecamp comments update --ids-only type=bool
FLAG basecamp comments update --in type=string
FLAG basecamp comments update --json type=bool
FLAG basecamp comments update --markdown type=bool
FLAG basecamp comments update --md type=bool
Expand Down
11 changes: 11 additions & 0 deletions internal/commands/comment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,18 @@ import (

// NewCommentsCmd creates the comments command group (list/show/update).
func NewCommentsCmd() *cobra.Command {
var project string
Comment thread
jeremy marked this conversation as resolved.

cmd := &cobra.Command{
Use: "comments",
Short: "List and manage comments",
Long: "List, show, and update comments on items.",
Annotations: map[string]string{"agent_notes": "Comments are flat — reply to parent item, not to other comments\nURL fragments (#__recording_456) are comment IDs — comment on the parent recording_id, not the comment_id\nComments are on items (todos, messages, cards, etc.) — not on other comments"},
}

cmd.PersistentFlags().StringVarP(&project, "project", "p", "", "Project ID or name")
cmd.PersistentFlags().StringVar(&project, "in", "", "Project ID (alias for --project)")

Comment thread
jeremy marked this conversation as resolved.
cmd.AddCommand(
newCommentsListCmd(),
newCommentsShowCmd(),
Expand Down Expand Up @@ -247,9 +252,15 @@ You can pass either a comment ID or a Basecamp URL:

// NewCommentCmd creates the 'comment' shortcut (alias for 'comments create').
func NewCommentCmd() *cobra.Command {
var project string

cmd := newCommentsCreateCmd()
cmd.Use = "comment <id|url> <content>"
cmd.Short = "Add a comment (shortcut for 'comments create')"

cmd.Flags().StringVarP(&project, "project", "p", "", "Project ID or name")
cmd.Flags().StringVar(&project, "in", "", "Project ID (alias for --project)")

Comment thread
jeremy marked this conversation as resolved.
return cmd
}

Expand Down
58 changes: 58 additions & 0 deletions internal/commands/comment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package commands

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// TestCommentShortcutAcceptsInFlag tests that the top-level 'comment' shortcut
// accepts --in, matching the 'comments' group. Previously, 'comment' was built
// directly from newCommentsCreateCmd() and did not inherit the persistent flags
// registered on NewCommentsCmd().
func TestCommentShortcutAcceptsInFlag(t *testing.T) {
app, _ := setupTestApp(t)
app.Config.ProjectID = "123"

cmd := NewCommentCmd()

// --in should be accepted (not "unknown flag"). The command will proceed
// to RunE and hit an API/network error, which is fine — we're testing
// flag acceptance, not API behavior.
err := executeCommand(cmd, app, "--in", "456", "789", "hello")

// If there's an error, it must NOT be "unknown flag"
require.NotNil(t, err)
assert.NotContains(t, err.Error(), "unknown flag")
assert.NotContains(t, err.Error(), "unknown shorthand")
}

// TestCommentShortcutAcceptsProjectFlag tests the -p shorthand works too.
func TestCommentShortcutAcceptsProjectFlag(t *testing.T) {
app, _ := setupTestApp(t)
app.Config.ProjectID = "123"

cmd := NewCommentCmd()

err := executeCommand(cmd, app, "-p", "456", "789", "hello")

require.NotNil(t, err)
assert.NotContains(t, err.Error(), "unknown flag")
assert.NotContains(t, err.Error(), "unknown shorthand")
}

// TestCommentsGroupAcceptsInFlag tests the 'comments' group accepts --in.
func TestCommentsGroupAcceptsInFlag(t *testing.T) {
app, _ := setupTestApp(t)
app.Config.ProjectID = "123"

cmd := NewCommentsCmd()

err := executeCommand(cmd, app, "list", "--in", "456", "789")

// Should not be "unknown flag" or "unknown shorthand"
require.NotNil(t, err)
assert.NotContains(t, err.Error(), "unknown flag")
assert.NotContains(t, err.Error(), "unknown shorthand")
}
81 changes: 79 additions & 2 deletions internal/names/resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type Resolver struct {
mu sync.RWMutex
projects []Project
people []Person
pingable []Person // cached /people/pingable.json
todolists map[string][]Todolist // keyed by project ID
me *Person // cached /my/profile.json result
}
Expand Down Expand Up @@ -80,6 +81,7 @@ func (r *Resolver) SetAccountID(accountID string) {
// Clear cache since data is account-specific
r.projects = nil
r.people = nil
r.pingable = nil
r.me = nil
r.todolists = make(map[string][]Todolist)
}
Expand Down Expand Up @@ -206,8 +208,48 @@ func (r *Resolver) ResolvePerson(ctx context.Context, input string) (string, str
return "", "", output.ErrAmbiguous("person", names)
}

// Not found - provide suggestions
suggestions := suggest(input, people, func(p Person) string { return p.Name })
// Fallback: try pingable people (/people/pingable.json) which includes
// people not in /people.json (e.g., clients, external collaborators).
// Degrade gracefully on error — the people list already provides suggestions.
pingable, _ := r.getPingable(ctx)

if len(pingable) > 0 {
// Try email exact match
for _, p := range pingable {
if strings.EqualFold(p.Email, input) {
return strconv.FormatInt(p.ID, 10), p.Name, nil
}
}

// Try name resolution
pingMatch, pingMatches := resolve(input, pingable, func(p Person) (int64, string) {
return p.ID, p.Name
})
if pingMatch != nil {
return strconv.FormatInt(pingMatch.ID, 10), pingMatch.Name, nil
}
if len(pingMatches) > 1 {
pingNames := make([]string, len(pingMatches))
for i, m := range pingMatches {
pingNames[i] = m.Name
}
return "", "", output.ErrAmbiguous("person", pingNames)
}
}

// Not found - provide suggestions from both lists (deduplicated by ID)
seen := make(map[int64]struct{}, len(people))
allPeople := make([]Person, 0, len(people)+len(pingable))
for _, p := range people {
seen[p.ID] = struct{}{}
allPeople = append(allPeople, p)
}
for _, p := range pingable {
if _, ok := seen[p.ID]; !ok {
allPeople = append(allPeople, p)
}
}
suggestions := suggest(input, allPeople, func(p Person) string { return p.Name })
if len(suggestions) > 0 {
return "", "", output.ErrNotFoundHint("Person", input, "Did you mean: "+strings.Join(suggestions, ", "))
}
Expand Down Expand Up @@ -268,6 +310,7 @@ func (r *Resolver) ClearCache() {
defer r.mu.Unlock()
r.projects = nil
r.people = nil
r.pingable = nil
r.me = nil
r.todolists = make(map[string][]Todolist)
}
Expand Down Expand Up @@ -372,6 +415,40 @@ func (r *Resolver) getPeople(ctx context.Context) ([]Person, error) {
return people, nil
}

func (r *Resolver) getPingable(ctx context.Context) ([]Person, error) {
r.mu.RLock()
if r.pingable != nil {
defer r.mu.RUnlock()
return r.pingable, nil
}
r.mu.RUnlock()

r.mu.Lock()
defer r.mu.Unlock()

// Double-check after acquiring write lock
if r.pingable != nil {
return r.pingable, nil
}

sdkPeople, err := r.forAccount().People().Pingable(ctx)
if err != nil {
return nil, convertSDKError(err)
}

pingable := make([]Person, 0, len(sdkPeople))
for _, p := range sdkPeople {
pingable = append(pingable, Person{
ID: p.ID,
Name: p.Name,
Email: p.EmailAddress,
})
}

r.pingable = pingable
return pingable, nil
}

func (r *Resolver) getTodolists(ctx context.Context, projectID string) ([]Todolist, error) {
r.mu.RLock()
if lists, ok := r.todolists[projectID]; ok {
Expand Down
Loading
Loading