Skip to content
Merged
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
6 changes: 3 additions & 3 deletions internal/commands/cards.go
Original file line number Diff line number Diff line change
Expand Up @@ -982,7 +982,7 @@ Use - as the body argument to read the body from stdin:
if err != nil {
return err
}
mentionResult, mentionErr := resolveMentions(cmd.Context(), app.Names, content)
mentionResult, mentionErr := resolveMentions(cmd.Context(), app.Names, mentionScope(app, resolvedProjectID), content)
if mentionErr != nil {
return mentionErr
}
Expand Down Expand Up @@ -1101,7 +1101,7 @@ You can pass either a card ID or a Basecamp URL:
}

// Extract ID from URL if provided
cardIDStr := extractID(args[0])
cardIDStr, urlProjectID := extractWithProject(args[0])

cardID, err := strconv.ParseInt(cardIDStr, 10, 64)
if err != nil {
Expand Down Expand Up @@ -1157,7 +1157,7 @@ You can pass either a card ID or a Basecamp URL:
if err != nil {
return err
}
mentionResult, mentionErr := resolveMentions(cmd.Context(), app.Names, html)
mentionResult, mentionErr := resolveMentions(cmd.Context(), app.Names, mentionScope(app, urlProjectID, projectFlagValue(cmd), app.Flags.Project), html)
if mentionErr != nil {
return mentionErr
}
Expand Down
4 changes: 2 additions & 2 deletions internal/commands/chat.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ func runChatPost(cmd *cobra.Command, app *appctx.App, chatID, project, content,
if contentType == "" {
mentionInput = richtext.MarkdownToHTML(content)
}
result, resolveErr := resolveMentions(cmd.Context(), app.Names, mentionInput)
result, resolveErr := resolveMentions(cmd.Context(), app.Names, mentionScope(app, resolvedProjectID, project, app.Flags.Project), mentionInput)
if resolveErr != nil {
return resolveErr
}
Expand Down Expand Up @@ -1025,7 +1025,7 @@ edit to rich text.`,
if ct == "" {
messageContent = richtext.MarkdownToHTML(messageContent)
}
result, resolveErr := resolveMentions(cmd.Context(), app.Names, messageContent)
result, resolveErr := resolveMentions(cmd.Context(), app.Names, mentionScope(app, urlProjectID, resolvedProjectID, *project, app.Flags.Project), messageContent)
if resolveErr != nil {
return resolveErr
}
Expand Down
3 changes: 3 additions & 0 deletions internal/commands/chat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,9 @@ func (t *mockChatMultiMentionTransport) RoundTrip(req *http.Request) (*http.Resp
switch {
case strings.Contains(req.URL.Path, "/projects.json"):
body = `[{"id": 123, "name": "Test Project"}]`
case strings.HasSuffix(req.URL.Path, "/projects/123/people.json"):
// The project's people, where an unpingable agent would be found.
body = `[{"id": 42000, "name": "Jane Smith", "attachable_sgid": "sgid-jane", "personable_type": "User"}]`
case strings.Contains(req.URL.Path, "/projects/"):
body = `{"id": 123, "dock": [{"name": "chat", "id": 789, "enabled": true}]}`
case strings.Contains(req.URL.Path, "/circles/people.json") || strings.Contains(req.URL.Path, "/people/pingable.json"):
Expand Down
6 changes: 3 additions & 3 deletions internal/commands/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -1051,7 +1051,7 @@ as backslash-n.`,

// Extract comment ID from URL if provided
// Uses extractCommentWithProject to prefer CommentID from URL fragments
commentIDStr, _ := extractCommentWithProject(args[0])
commentIDStr, urlProjectID := extractCommentWithProject(args[0])

commentID, err := strconv.ParseInt(commentIDStr, 10, 64)
if err != nil {
Expand Down Expand Up @@ -1084,7 +1084,7 @@ as backslash-n.`,
}

// Resolve @mentions
mentionResult, err := resolveMentions(cmd.Context(), app.Names, html)
mentionResult, err := resolveMentions(cmd.Context(), app.Names, mentionScope(app, urlProjectID, projectFlagValue(cmd), app.Flags.Project), html)
if err != nil {
return err
}
Expand Down Expand Up @@ -1231,7 +1231,7 @@ busybox-ash) it posts a literal leading $ and keeps \n as backslash-n:
}

// Resolve @mentions (e.g., @John, @John.Doe → clickable mention tags)
mentionResult, err := resolveMentions(cmd.Context(), app.Names, html)
mentionResult, err := resolveMentions(cmd.Context(), app.Names, batchMentionScope(cmd, app, recordingArg), html)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/commands/comment_thread_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func TestFocusMentionRoundTripsThroughResolveMentions(t *testing.T) {
// embedded SGID must resolve to a mention attachment with zero
// lookups — even when the display label came from a hostile name.
html := richtext.MarkdownToHTML(syntax)
result, err := resolveMentions(context.Background(), nil, html)
result, err := resolveMentions(context.Background(), nil, nil, html)
require.NoError(t, err)
assert.Contains(t, result.HTML, "BAh7CEkiCG")
assert.Contains(t, result.HTML, "application/vnd.basecamp.mention")
Expand Down
141 changes: 135 additions & 6 deletions internal/commands/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -632,19 +632,32 @@ func applySubscribeFlags(ctx context.Context, resolver *names.Resolver, subscrib
// resolveMentions scans HTML for mention syntax and replaces matches with
// Basecamp mention attachment tags. Supports three syntaxes:
// - [@Name](mention:SGID) — zero API calls (SGID embedded directly)
// - [@Name](person:ID) — one API call (ID→SGID via pingable set)
// - @Name / @First.Last — fuzzy name resolution via pingable set
// - [@Name](person:ID) — one API call (ID→SGID via pingable set; an agent's
// ID costs one more, a person lookup)
// - @Name / @First.Last — fuzzy name resolution via pingable set, then the
// agents on the project in scope
//
// Also supports @sgid:VALUE inline syntax for pipeline composability.
// Silently returns unchanged HTML if no mentions are found.
//
// Agents cannot be pinged, so the pingable set never holds them. scope names
// the project whose agents a fuzzy @Name may reach; it is consulted only when
// the pingable set has no exact answer, and may be nil when the command has
// no project in scope.
//
// Fuzzy @Name mentions that cannot be resolved (not found or ambiguous) are
// left as plain text; their names are returned in the Unresolved slice.
// Deterministic syntaxes (mention:SGID, person:ID) still hard-fail on error.
func resolveMentions(ctx context.Context, resolver *names.Resolver, html string) (richtext.MentionResult, error) {
func resolveMentions(ctx context.Context, resolver *names.Resolver, scope names.ProjectScope, html string) (richtext.MentionResult, error) {
return richtext.ResolveMentions(html,
func(name string) (string, string, error) {
person, err := resolver.ResolvePersonByName(ctx, name)
person, err := resolver.ResolveMentionByName(ctx, name, scope)
Comment thread
jeremy marked this conversation as resolved.
// A project that cannot be resolved is the command's error, not a
// missed mention: never downgrade it to plain text.
var scopeErr *names.ScopeError
if errors.As(err, &scopeErr) {
return "", "", scopeErr.Err
}
if err != nil {
// Downgrade resolver-level not-found and ambiguous to skip — leave
// as plain text. Only match errors without an HTTP status; API-level
Expand All @@ -667,7 +680,7 @@ func resolveMentions(ctx context.Context, resolver *names.Resolver, html string)
if err != nil {
return "", "", output.ErrUsage(fmt.Sprintf("invalid person ID %q — must be numeric", id))
}
person, err := resolver.ResolvePersonByID(ctx, personID)
person, err := resolver.ResolveMentionByID(ctx, personID)
if err != nil {
return "", "", err
}
Expand All @@ -679,12 +692,128 @@ func resolveMentions(ctx context.Context, resolver *names.Resolver, html string)
)
}

// mentionScope returns the project a fuzzy @mention may draw agents from: the
// first non-blank candidate, in the caller's order of precedence — typically
// a project the command already resolved, a URL's bucket, then --in. A
// numeric ID is used as-is; a name is resolved only when the scope is
// consulted. Returns nil when every candidate is blank.
func mentionScope(app *appctx.App, candidates ...string) names.ProjectScope {
for _, candidate := range candidates {
candidate = strings.TrimSpace(candidate)
if candidate == "" {
continue
}
if scope := names.ParseProjectScope(candidate); scope != nil {
return scope
}
return func(ctx context.Context) (int64, error) {
id, _, err := app.Names.ResolveProject(ctx, candidate)
if err != nil {
return 0, err
}
return strconv.ParseInt(id, 10, 64)
}
}
return nil
}

// sameProjectID compares two project IDs as numbers, so "00123" and "123"
// name the same project. Non-numeric values compare as text.
func sameProjectID(a, b string) bool {
x, errA := strconv.ParseInt(a, 10, 64)
y, errB := strconv.ParseInt(b, 10, 64)
if errA != nil || errB != nil {
return a == b
}
return x == y
}

// batchMentionScope returns the mention scope for a comments create target
// list. One resolved mention goes to every target, so the scope must be a
// project every target is known to be in; anything less leaves agents out of
// scope and a fuzzy agent mention stays text, with the unresolved notice. It
// never refuses the command: which targets are posted to is decided exactly
// as before, with a URL's bucket winning over --in.
//
// - Every target a URL in one bucket: that bucket.
// - Only bare IDs: --in, if given.
// - URLs in one bucket plus bare IDs: that bucket, if --in names it too
// (--in is what vouches for the bare IDs).
// - URLs in different buckets: none.
//
// Tokens that are neither a URL nor a number are skipped by the posting loop,
// so they are skipped here too. A named --in is resolved only if the scope is
// consulted.
func batchMentionScope(cmd *cobra.Command, app *appctx.App, targets string) names.ProjectScope {
urlProject, bare := "", false
for part := range strings.SplitSeq(targets, ",") {
part = strings.TrimSpace(part)
if part == "" {
continue
}
recordingID, projectID := extractWithProject(part)
if _, err := strconv.ParseInt(recordingID, 10, 64); err != nil {
continue
}
switch {
case projectID == "":
bare = true
Comment thread
jeremy marked this conversation as resolved.
case urlProject != "" && !sameProjectID(urlProject, projectID):
return nil
default:
urlProject = projectID
}
}

explicit := projectFlagValue(cmd)
if explicit == "" {
explicit = app.Flags.Project
}
switch {
case urlProject == "":
return mentionScope(app, explicit)
case !bare:
return mentionScope(app, urlProject)
case explicit == "":
return nil
}

vouched := mentionScope(app, explicit)
if vouched == nil {
return nil
}
return func(ctx context.Context) (int64, error) {
id, err := vouched(ctx)
Comment thread
jeremy marked this conversation as resolved.
if err != nil {
return 0, err
}
if !sameProjectID(strconv.FormatInt(id, 10), urlProject) {
return 0, nil
}
return id, nil
}
}

// unresolvedMentionWarning formats a warning string for unresolved mentions.
// The hint names the one way a fuzzy @Name reaches an agent, since a
// mention meant for an agent is the miss a reader cannot otherwise explain.
func unresolvedMentionWarning(unresolved []string) string {
if len(unresolved) == 0 {
return ""
}
return "Unresolved mentions left as text: " + strings.Join(unresolved, ", ")
return "Unresolved mentions left as text: " + strings.Join(unresolved, ", ") +
" (agents match by name only among the people on the command's project; name it with --in or a URL if the command has none; [@Name](person:ID) needs no project)"
}

// projectFlagValue returns the --project/--in value visible to cmd, including
// one a parent command group defines, or "" when neither flag exists.
func projectFlagValue(cmd *cobra.Command) string {
for _, name := range []string{"project", "in"} {
if f := cmd.Flags().Lookup(name); f != nil && f.Value.String() != "" {
return f.Value.String()
}
}
return ""
}

// projectFlagChanged reports whether the user explicitly passed --project or
Expand Down
Loading
Loading