diff --git a/internal/commands/people.go b/internal/commands/people.go index 787041bc9..de0a3579f 100644 --- a/internal/commands/people.go +++ b/internal/commands/people.go @@ -40,6 +40,20 @@ type MePerson struct { ID int64 `json:"id"` Name string `json:"name"` Email string `json:"email"` + + // PersonableType and Boss are reported for an agent profile only. + // PersonableType is what Basecamp says the person is ("Agent" for a + // connected agent); Boss is the person a personal agent works for, as + // Basecamp names them. Together they let a caller check that a profile + // is an agent, and whose, without a second lookup. + PersonableType string `json:"personable_type,omitempty"` + Boss *MeBoss `json:"boss,omitempty"` +} + +// MeBoss is who a personal agent works for. +type MeBoss struct { + ID int64 `json:"id"` + Name string `json:"name"` } // AccountInfo represents an account in the me command output @@ -55,8 +69,12 @@ func NewMeCmd() *cobra.Command { cmd := &cobra.Command{ Use: "me", Short: "Show current user profile", - Long: "Display information about the currently authenticated user.", - RunE: runMe, + Long: `Display information about the currently authenticated user. + +On an agent profile there is no identity: me shows the agent's person in the +account it is bound to, its personable_type, and, for a personal agent, its +boss, the person it works for.`, + RunE: runMe, } return cmd } @@ -190,31 +208,63 @@ func agentProfile(app *appctx.App) bool { return os.Getenv("BASECAMP_TOKEN") == "" && app.Auth.GetOAuthType() == "agent" } +// agentProfileDocument is the part of an agent's /my/profile.json that `me` +// reports. The SDK's Person has no boss, so the document is read through the +// account client's plain GET — the one `basecamp api get` makes — in the one +// request `me` already spends. +type agentProfileDocument struct { + ID int64 `json:"id"` + Name string `json:"name"` + EmailAddress string `json:"email_address"` + PersonableType string `json:"personable_type"` + Boss *struct { + ID int64 `json:"id"` + Name string `json:"name"` + } `json:"boss"` +} + // runMeAgent shows the agent an agent profile authenticates as: its person // record in the account its credential is bound to, which is the only -// account it can reach. +// account it can reach, and whom it works for when Basecamp says. func runMeAgent(cmd *cobra.Command, app *appctx.App) error { if err := app.RequireAccount(); err != nil { return err } - p, err := app.Account().People().Me(cmd.Context()) + resp, err := app.Account().Get(cmd.Context(), "/my/profile.json") if err != nil { return convertSDKError(err) } + var p agentProfileDocument + if err := resp.UnmarshalData(&p); err != nil { + return fmt.Errorf("reading the agent's profile: %w", err) + } accountID, err := strconv.ParseInt(app.Config.AccountID, 10, 64) if err != nil { return output.ErrUsage(fmt.Sprintf("Invalid account ID %q", app.Config.AccountID)) } + person := &MePerson{ID: p.ID, Name: p.Name, Email: p.EmailAddress, PersonableType: p.PersonableType} + // A boss without an id is not one anybody can check against, so it is + // left out rather than reported as person 0. + if p.Boss != nil && p.Boss.ID > 0 { + person.Boss = &MeBoss{ID: p.Boss.ID, Name: p.Boss.Name} + } result := MeOutput{ Accounts: []AccountInfo{{ID: accountID, Current: true}}, - Person: &MePerson{ID: p.ID, Name: p.Name, Email: p.EmailAddress}, + Person: person, } label := meLabel(0, p.Name, p.EmailAddress) if p.Name == "" && p.EmailAddress == "" { label = fmt.Sprintf("person %d", p.ID) } summary := fmt.Sprintf("%s - agent in Basecamp account %d", label, accountID) + if person.Boss != nil { + boss := person.Boss.Name + if strings.TrimSpace(boss) == "" { + boss = fmt.Sprintf("person %d", person.Boss.ID) + } + summary += ", working for " + boss + } return app.OK(result, output.WithSummary(summary), diff --git a/internal/commands/people_test.go b/internal/commands/people_test.go index 7c5920f31..024b775b6 100644 --- a/internal/commands/people_test.go +++ b/internal/commands/people_test.go @@ -1402,3 +1402,96 @@ func TestMeOnAnAgentProfileReadsTheAgentsPerson(t *testing.T) { assert.JSONEq(t, `{"id":777,"name":"Triage Bot","email":"bot@example.com"}`, string(envelope.Data["person"])) assert.JSONEq(t, `[{"id":555,"name":"","href":"","app_href":"","current":true}]`, string(envelope.Data["accounts"])) } + +// runMeOnAgentProfile runs `me --json` on a stored agent credential against +// a server whose /555/my/profile.json answers profile, and returns the +// envelope's summary and data. +func runMeOnAgentProfile(t *testing.T, profile map[string]any) (string, map[string]json.RawMessage) { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if r.URL.Path != "/555/my/profile.json" { + w.WriteHeader(http.StatusUnauthorized) + return + } + json.NewEncoder(w).Encode(profile) + })) + t.Cleanup(server.Close) + + t.Setenv("BASECAMP_TOKEN", "") + t.Setenv("BASECAMP_NO_KEYRING", "1") + tmpDir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", tmpDir) + + cfg := &config.Config{AccountID: "555", BaseURL: server.URL, CacheDir: t.TempDir()} + authMgr := auth.NewManager(cfg, nil) + authMgr.SetStore(auth.NewStore(filepath.Join(tmpDir, "basecamp"))) + require.NoError(t, authMgr.GetStore().Save(config.NormalizeBaseURL(server.URL), &auth.Credentials{ + AccessToken: "bc_at_agent", + OAuthType: "agent", + ClientID: "agent-client", + ClientSecret: "agent-secret", + TokenEndpoint: server.URL + "/oauth/tokens", + ExpiresAt: 9999999999, + })) + + buf := &bytes.Buffer{} + sdkClient := basecamp.NewClient(&basecamp.Config{BaseURL: server.URL}, &peopleTestTokenProvider{}, basecamp.WithMaxRetries(1)) + app := &appctx.App{ + Config: cfg, + Auth: authMgr, + SDK: sdkClient, + Names: names.NewResolver(sdkClient, authMgr, cfg.AccountID), + Output: output.New(output.Options{Format: output.FormatJSON, Writer: buf}), + } + require.NoError(t, executePeopleCommand(NewMeCmd(), app)) + + var envelope struct { + Summary string `json:"summary"` + Data map[string]json.RawMessage `json:"data"` + } + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope), buf.String()) + return envelope.Summary, envelope.Data +} + +// TestMeOnAPersonalAgentNamesItsBoss: a personal agent's profile names the +// person it works for, and `me` reports it with the personable type, so a +// caller can check "this profile is an agent, and it is mine" from one +// command instead of a second lookup. +func TestMeOnAPersonalAgentNamesItsBoss(t *testing.T) { + summary, data := runMeOnAgentProfile(t, map[string]any{ + "id": 777, "name": "Jane Doe (agent)", "email_address": "", + "personable_type": "Agent", + "boss": map[string]any{"id": 42, "name": "Jane Doe", "email_address": "jane@example.com"}, + }) + assert.Equal(t, "Jane Doe (agent) - agent in Basecamp account 555, working for Jane Doe", summary) + assert.JSONEq(t, + `{"id":777,"name":"Jane Doe (agent)","email":"","personable_type":"Agent","boss":{"id":42,"name":"Jane Doe"}}`, + string(data["person"])) +} + +// TestMeOnAnAgentWithNoBossReportsNone: a shared agent works for no one in +// particular, and a boss Basecamp names without an id is not one anyone can +// check against; neither is reported, and neither is invented as person 0. +func TestMeOnAnAgentWithNoBossReportsNone(t *testing.T) { + for name, profile := range map[string]map[string]any{ + "no boss": {"id": 777, "name": "Triage Bot", "personable_type": "Agent"}, + "null boss": {"id": 777, "name": "Triage Bot", "personable_type": "Agent", "boss": nil}, + "boss, no id": {"id": 777, "name": "Triage Bot", "personable_type": "Agent", "boss": map[string]any{"name": "Someone"}}, + } { + t.Run(name, func(t *testing.T) { + summary, data := runMeOnAgentProfile(t, profile) + assert.Equal(t, "Triage Bot - agent in Basecamp account 555", summary) + assert.JSONEq(t, `{"id":777,"name":"Triage Bot","email":"","personable_type":"Agent"}`, string(data["person"])) + }) + } +} + +// TestMeOnAnAgentWhoseBossHasNoNameSaysWhichPerson: the summary names the +// boss by id rather than trailing off when the name is blank. +func TestMeOnAnAgentWhoseBossHasNoNameSaysWhichPerson(t *testing.T) { + summary, _ := runMeOnAgentProfile(t, map[string]any{ + "id": 777, "name": "Jane Doe (agent)", "personable_type": "Agent", "boss": map[string]any{"id": 42, "name": " "}, + }) + assert.Equal(t, "Jane Doe (agent) - agent in Basecamp account 555, working for person 42", summary) +} diff --git a/skills/basecamp-connect/SKILL.md b/skills/basecamp-connect/SKILL.md index 4661bfbf7..02dd5a59f 100644 --- a/skills/basecamp-connect/SKILL.md +++ b/skills/basecamp-connect/SKILL.md @@ -317,7 +317,9 @@ the person must decide (a repo to pick, a mention that couldn't be posted). **Identity.** Never set up a profile whose identity the person hasn't confirmed. Before the first setup, run `basecamp me -P '' --json` and say who it is: `identity` (name, email) and, when present, `person.name` and -`person.id`. Go on only when they say that is the agent. If it names someone +`person.id`. An agent credential has no `identity`; its `person` carries +`personable_type` and, for a personal agent, `boss`, the person it works for. +Go on only when they say that is the agent. If it names someone else, stop: don't run setup and don't reconnect. **Writing to Basecamp.** Everything this skill and its subagents post (boosts,