From 89383ddde52bda0e40d104fb79b47189215bc37b Mon Sep 17 00:00:00 2001 From: Holger Selover-Stephan Date: Thu, 30 Jul 2026 16:56:19 +0200 Subject: [PATCH 1/2] feat(user): list all users and expose identity fields for merge triage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidating duplicate accounts needed two things the CLI could not do: see the whole user population, and see which credential each account actually authenticates with. `user list` (aliases: search, ls) now takes an OPTIONAL query. Omitted, it requests the server's unfiltered listing — the one a platform ADMIN/OWNER may run. `$query` became a nullable String with omitempty so a nil pointer drops the variable entirely, which GraphQL coerces to `filter: {}`; sending an empty string would instead be a blank filter matching nothing. A caller without platform authority gets an empty list rather than an error, so the human branch explains why instead of rendering a bare table. Without an explicit --limit/--offset the listing pages to exhaustion via api.CollectAll, per the whole-corpus rule — a population larger than one 200-row server page must not be silently truncated when the whole point is finding duplicates. UserFields gained identityProvider, githubId, externalId, externalAppId and linkedAt, and PROVIDER joins the table. These predict what `user merge` does to a login: mergeUsers adopts a source provider id only where the target's is null, and email is target-wins with the source's nulled, so whether a credential survives is knowable before an irreversible mutation with no dry-run. agentic-usage.md now says so next to `user merge`, along with the fact that the target's handle always wins and there is no admin handle rename. Co-Authored-By: Claude Opus 5 --- docs/plans/user-list-identity-fields.md | 90 ++++++ internal/api/gen/generated.go | 365 +++++++++++++++++++++++- internal/api/queries/org.graphql | 20 +- internal/cmd/agentic/agentic-usage.md | 22 +- internal/cmd/list_naming_test.go | 1 + internal/cmd/user/user.go | 131 ++++++--- internal/cmd/user_cmd_test.go | 150 ++++++++++ internal/cmdutil/userref.go | 4 +- 8 files changed, 731 insertions(+), 52 deletions(-) create mode 100644 docs/plans/user-list-identity-fields.md diff --git a/docs/plans/user-list-identity-fields.md b/docs/plans/user-list-identity-fields.md new file mode 100644 index 0000000..fc572c1 --- /dev/null +++ b/docs/plans/user-list-identity-fields.md @@ -0,0 +1,90 @@ +# `hadron user list` — full user listing + identity fields for merge triage + +Design-as-built for the two gaps that surfaced while consolidating duplicate +production accounts with `user merge`. + +## Why + +Duplicate accounts are created by ordinary behaviour: signing in once with +GitHub and once with Google/Apple/email mints two Users. Consolidating them +needs two things the CLI could not do. + +**1. See the whole population.** The server's `users` query already lets a +platform `ADMIN`/`OWNER` omit `filter.query` for the full list +(`cor:acl:070:02`), but `SearchUsers` declared `$query: String!` and the command +rejected an empty one — so the only way to find duplicates was to guess +substrings and union the results. That finds the accounts you thought to search +for, which is the wrong set: an account with an auto-generated hex handle and no +GitHub username (exactly what an email signup produces) is invisible unless you +already know its address. + +**2. See which credential each account authenticates with.** `UserFields` +selected `email` and `githubUsername` only. That is not enough to predict what a +merge does to a login, and `mergeUsers` is irreversible with no dry-run. + +## Surface + +``` +hadron user list [query] [--limit N] [--offset N] [--json] # aliases: search, ls +``` + +- Omitting the query requests the unfiltered listing. A caller without platform + authority gets an **empty list, not an error** — the server will not confirm + that accounts it won't show you exist, and the CLI must not turn that into a + distinguishable signal. The human branch prints a hint so an empty table isn't + read as "no users". +- Without an explicit `--limit`/`--offset` the listing pages to exhaustion via + `api.CollectAll`, per the whole-corpus rule in CLAUDE.md. Either flag selects a + single explicit page. +- `UserFields` gained `identityProvider`, `githubId`, `externalId`, + `externalAppId`, `linkedAt`; `PROVIDER` joins the table. + +## Decisions + +**Nullable `$query` + omitempty, not an empty string.** A blank query and an +omitted one are different requests: blank is a filter that matches nothing, +omitted is "no filter". `# @genqlient(omitempty: true)` on a `String` drops the +variable, and GraphQL input-object coercion renders the absent variable as +`filter: {}` — the admin full-list path. Sending `""` would have quietly +returned an empty page and looked like a permissions problem. The command +still rejects an explicitly empty argument as usage, since `user list ""` is a +typo, not a request to enumerate the platform. + +**The identity fields go on the shared `UserFields` fragment**, so `org member +list` and `access check` surface them too. They are the fields that distinguish +"two rows for one human" from "two humans", and that judgement is made wherever +users are listed, not only in `user list`. + +**`identityProvider` is displayed but is not the answer.** It is tempting to +read it as the login method; it is not. Auth resolves a user by *provider id* +first (`googleId` / `githubId` / `appleId`), then by verified email — +`identityProvider` only records which provider created the row, and the linking +branch explicitly keeps the existing value. `mergeUsers` mirrors that: it adopts +each provider id when the target's is null (`target.googleId ?? source.googleId`), +so a merge usually *preserves* both sign-in paths. The field that actually gets +destroyed is `email`, which is target-wins and nulled on the source tombstone. +The column is worth showing because it explains an account's origin, but the +triage question is "is the target's corresponding id column empty?". + +**No admin handle rename, so the target choice is final.** `profile set` is +self-only and `mergeUsers` keeps `target.handle ?? source.handle` — with handle +NOT NULL the target's always wins. Whichever account you merge *into* keeps its +handle forever, which is why the auto-generated hex handles have to be sources. +`agentic-usage.md` says this outright next to `user merge`; it is the kind of +thing you only discover after the irreversible step. + +## Tests + +`internal/cmd/user_cmd_test.go` — the query variable is omitted (not empty) for +the full list; identity fields round-trip through `--json`; the unbounded +listing drains multiple pages and stops at `total`; an explicit `--limit` makes +exactly one request; an empty full list explains itself. + +`list_naming_test.go` gains the `user ls`/`user list` alias pair. + +## Not done + +`googleId` and `appleId` are columns on the server's User but are not exposed on +the GraphQL type, so a Google- or Apple-origin account's provider id is still +invisible to the CLI — it has to be inferred from `identityProvider`. Adding +them is a hadron-server change; worth doing before the next consolidation. diff --git a/internal/api/gen/generated.go b/internal/api/gen/generated.go index 6f4425e..7bfea29 100644 --- a/internal/api/gen/generated.go +++ b/internal/api/gen/generated.go @@ -443,6 +443,23 @@ func (v *AddOrgMemberAddOrgMemberUser) GetGithubUsername() *string { // GetRoles returns AddOrgMemberAddOrgMemberUser.Roles, and is useful for accessing the field via an interface. func (v *AddOrgMemberAddOrgMemberUser) GetRoles() []Role { return v.UserFields.Roles } +// GetIdentityProvider returns AddOrgMemberAddOrgMemberUser.IdentityProvider, and is useful for accessing the field via an interface. +func (v *AddOrgMemberAddOrgMemberUser) GetIdentityProvider() *string { + return v.UserFields.IdentityProvider +} + +// GetGithubId returns AddOrgMemberAddOrgMemberUser.GithubId, and is useful for accessing the field via an interface. +func (v *AddOrgMemberAddOrgMemberUser) GetGithubId() *int { return v.UserFields.GithubId } + +// GetExternalId returns AddOrgMemberAddOrgMemberUser.ExternalId, and is useful for accessing the field via an interface. +func (v *AddOrgMemberAddOrgMemberUser) GetExternalId() *string { return v.UserFields.ExternalId } + +// GetExternalAppId returns AddOrgMemberAddOrgMemberUser.ExternalAppId, and is useful for accessing the field via an interface. +func (v *AddOrgMemberAddOrgMemberUser) GetExternalAppId() *string { return v.UserFields.ExternalAppId } + +// GetLinkedAt returns AddOrgMemberAddOrgMemberUser.LinkedAt, and is useful for accessing the field via an interface. +func (v *AddOrgMemberAddOrgMemberUser) GetLinkedAt() *string { return v.UserFields.LinkedAt } + func (v *AddOrgMemberAddOrgMemberUser) UnmarshalJSON(b []byte) error { if string(b) == "null" { @@ -480,6 +497,16 @@ type __premarshalAddOrgMemberAddOrgMemberUser struct { GithubUsername *string `json:"githubUsername"` Roles []Role `json:"roles"` + + IdentityProvider *string `json:"identityProvider"` + + GithubId *int `json:"githubId"` + + ExternalId *string `json:"externalId"` + + ExternalAppId *string `json:"externalAppId"` + + LinkedAt *string `json:"linkedAt"` } func (v *AddOrgMemberAddOrgMemberUser) MarshalJSON() ([]byte, error) { @@ -499,6 +526,11 @@ func (v *AddOrgMemberAddOrgMemberUser) __premarshalJSON() (*__premarshalAddOrgMe retval.Handle = v.UserFields.Handle retval.GithubUsername = v.UserFields.GithubUsername retval.Roles = v.UserFields.Roles + retval.IdentityProvider = v.UserFields.IdentityProvider + retval.GithubId = v.UserFields.GithubId + retval.ExternalId = v.UserFields.ExternalId + retval.ExternalAppId = v.UserFields.ExternalAppId + retval.LinkedAt = v.UserFields.LinkedAt return &retval, nil } @@ -6644,6 +6676,21 @@ func (v *GetUserUser) GetGithubUsername() *string { return v.UserFields.GithubUs // GetRoles returns GetUserUser.Roles, and is useful for accessing the field via an interface. func (v *GetUserUser) GetRoles() []Role { return v.UserFields.Roles } +// GetIdentityProvider returns GetUserUser.IdentityProvider, and is useful for accessing the field via an interface. +func (v *GetUserUser) GetIdentityProvider() *string { return v.UserFields.IdentityProvider } + +// GetGithubId returns GetUserUser.GithubId, and is useful for accessing the field via an interface. +func (v *GetUserUser) GetGithubId() *int { return v.UserFields.GithubId } + +// GetExternalId returns GetUserUser.ExternalId, and is useful for accessing the field via an interface. +func (v *GetUserUser) GetExternalId() *string { return v.UserFields.ExternalId } + +// GetExternalAppId returns GetUserUser.ExternalAppId, and is useful for accessing the field via an interface. +func (v *GetUserUser) GetExternalAppId() *string { return v.UserFields.ExternalAppId } + +// GetLinkedAt returns GetUserUser.LinkedAt, and is useful for accessing the field via an interface. +func (v *GetUserUser) GetLinkedAt() *string { return v.UserFields.LinkedAt } + func (v *GetUserUser) UnmarshalJSON(b []byte) error { if string(b) == "null" { @@ -6681,6 +6728,16 @@ type __premarshalGetUserUser struct { GithubUsername *string `json:"githubUsername"` Roles []Role `json:"roles"` + + IdentityProvider *string `json:"identityProvider"` + + GithubId *int `json:"githubId"` + + ExternalId *string `json:"externalId"` + + ExternalAppId *string `json:"externalAppId"` + + LinkedAt *string `json:"linkedAt"` } func (v *GetUserUser) MarshalJSON() ([]byte, error) { @@ -6700,6 +6757,11 @@ func (v *GetUserUser) __premarshalJSON() (*__premarshalGetUserUser, error) { retval.Handle = v.UserFields.Handle retval.GithubUsername = v.UserFields.GithubUsername retval.Roles = v.UserFields.Roles + retval.IdentityProvider = v.UserFields.IdentityProvider + retval.GithubId = v.UserFields.GithubId + retval.ExternalId = v.UserFields.ExternalId + retval.ExternalAppId = v.UserFields.ExternalAppId + retval.LinkedAt = v.UserFields.LinkedAt return &retval, nil } @@ -8237,6 +8299,23 @@ func (v *MergeUsersMergeUsersUser) GetGithubUsername() *string { return v.UserFi // GetRoles returns MergeUsersMergeUsersUser.Roles, and is useful for accessing the field via an interface. func (v *MergeUsersMergeUsersUser) GetRoles() []Role { return v.UserFields.Roles } +// GetIdentityProvider returns MergeUsersMergeUsersUser.IdentityProvider, and is useful for accessing the field via an interface. +func (v *MergeUsersMergeUsersUser) GetIdentityProvider() *string { + return v.UserFields.IdentityProvider +} + +// GetGithubId returns MergeUsersMergeUsersUser.GithubId, and is useful for accessing the field via an interface. +func (v *MergeUsersMergeUsersUser) GetGithubId() *int { return v.UserFields.GithubId } + +// GetExternalId returns MergeUsersMergeUsersUser.ExternalId, and is useful for accessing the field via an interface. +func (v *MergeUsersMergeUsersUser) GetExternalId() *string { return v.UserFields.ExternalId } + +// GetExternalAppId returns MergeUsersMergeUsersUser.ExternalAppId, and is useful for accessing the field via an interface. +func (v *MergeUsersMergeUsersUser) GetExternalAppId() *string { return v.UserFields.ExternalAppId } + +// GetLinkedAt returns MergeUsersMergeUsersUser.LinkedAt, and is useful for accessing the field via an interface. +func (v *MergeUsersMergeUsersUser) GetLinkedAt() *string { return v.UserFields.LinkedAt } + func (v *MergeUsersMergeUsersUser) UnmarshalJSON(b []byte) error { if string(b) == "null" { @@ -8274,6 +8353,16 @@ type __premarshalMergeUsersMergeUsersUser struct { GithubUsername *string `json:"githubUsername"` Roles []Role `json:"roles"` + + IdentityProvider *string `json:"identityProvider"` + + GithubId *int `json:"githubId"` + + ExternalId *string `json:"externalId"` + + ExternalAppId *string `json:"externalAppId"` + + LinkedAt *string `json:"linkedAt"` } func (v *MergeUsersMergeUsersUser) MarshalJSON() ([]byte, error) { @@ -8293,6 +8382,11 @@ func (v *MergeUsersMergeUsersUser) __premarshalJSON() (*__premarshalMergeUsersMe retval.Handle = v.UserFields.Handle retval.GithubUsername = v.UserFields.GithubUsername retval.Roles = v.UserFields.Roles + retval.IdentityProvider = v.UserFields.IdentityProvider + retval.GithubId = v.UserFields.GithubId + retval.ExternalId = v.UserFields.ExternalId + retval.ExternalAppId = v.UserFields.ExternalAppId + retval.LinkedAt = v.UserFields.LinkedAt return &retval, nil } @@ -9436,6 +9530,29 @@ func (v *OrgMembersOrganizationMembersOrgMemberUser) GetGithubUsername() *string // GetRoles returns OrgMembersOrganizationMembersOrgMemberUser.Roles, and is useful for accessing the field via an interface. func (v *OrgMembersOrganizationMembersOrgMemberUser) GetRoles() []Role { return v.UserFields.Roles } +// GetIdentityProvider returns OrgMembersOrganizationMembersOrgMemberUser.IdentityProvider, and is useful for accessing the field via an interface. +func (v *OrgMembersOrganizationMembersOrgMemberUser) GetIdentityProvider() *string { + return v.UserFields.IdentityProvider +} + +// GetGithubId returns OrgMembersOrganizationMembersOrgMemberUser.GithubId, and is useful for accessing the field via an interface. +func (v *OrgMembersOrganizationMembersOrgMemberUser) GetGithubId() *int { return v.UserFields.GithubId } + +// GetExternalId returns OrgMembersOrganizationMembersOrgMemberUser.ExternalId, and is useful for accessing the field via an interface. +func (v *OrgMembersOrganizationMembersOrgMemberUser) GetExternalId() *string { + return v.UserFields.ExternalId +} + +// GetExternalAppId returns OrgMembersOrganizationMembersOrgMemberUser.ExternalAppId, and is useful for accessing the field via an interface. +func (v *OrgMembersOrganizationMembersOrgMemberUser) GetExternalAppId() *string { + return v.UserFields.ExternalAppId +} + +// GetLinkedAt returns OrgMembersOrganizationMembersOrgMemberUser.LinkedAt, and is useful for accessing the field via an interface. +func (v *OrgMembersOrganizationMembersOrgMemberUser) GetLinkedAt() *string { + return v.UserFields.LinkedAt +} + func (v *OrgMembersOrganizationMembersOrgMemberUser) UnmarshalJSON(b []byte) error { if string(b) == "null" { @@ -9473,6 +9590,16 @@ type __premarshalOrgMembersOrganizationMembersOrgMemberUser struct { GithubUsername *string `json:"githubUsername"` Roles []Role `json:"roles"` + + IdentityProvider *string `json:"identityProvider"` + + GithubId *int `json:"githubId"` + + ExternalId *string `json:"externalId"` + + ExternalAppId *string `json:"externalAppId"` + + LinkedAt *string `json:"linkedAt"` } func (v *OrgMembersOrganizationMembersOrgMemberUser) MarshalJSON() ([]byte, error) { @@ -9492,6 +9619,11 @@ func (v *OrgMembersOrganizationMembersOrgMemberUser) __premarshalJSON() (*__prem retval.Handle = v.UserFields.Handle retval.GithubUsername = v.UserFields.GithubUsername retval.Roles = v.UserFields.Roles + retval.IdentityProvider = v.UserFields.IdentityProvider + retval.GithubId = v.UserFields.GithubId + retval.ExternalId = v.UserFields.ExternalId + retval.ExternalAppId = v.UserFields.ExternalAppId + retval.LinkedAt = v.UserFields.LinkedAt return &retval, nil } @@ -11233,6 +11365,25 @@ func (v *SearchUsersUsersUsersPageItemsUser) GetGithubUsername() *string { // GetRoles returns SearchUsersUsersUsersPageItemsUser.Roles, and is useful for accessing the field via an interface. func (v *SearchUsersUsersUsersPageItemsUser) GetRoles() []Role { return v.UserFields.Roles } +// GetIdentityProvider returns SearchUsersUsersUsersPageItemsUser.IdentityProvider, and is useful for accessing the field via an interface. +func (v *SearchUsersUsersUsersPageItemsUser) GetIdentityProvider() *string { + return v.UserFields.IdentityProvider +} + +// GetGithubId returns SearchUsersUsersUsersPageItemsUser.GithubId, and is useful for accessing the field via an interface. +func (v *SearchUsersUsersUsersPageItemsUser) GetGithubId() *int { return v.UserFields.GithubId } + +// GetExternalId returns SearchUsersUsersUsersPageItemsUser.ExternalId, and is useful for accessing the field via an interface. +func (v *SearchUsersUsersUsersPageItemsUser) GetExternalId() *string { return v.UserFields.ExternalId } + +// GetExternalAppId returns SearchUsersUsersUsersPageItemsUser.ExternalAppId, and is useful for accessing the field via an interface. +func (v *SearchUsersUsersUsersPageItemsUser) GetExternalAppId() *string { + return v.UserFields.ExternalAppId +} + +// GetLinkedAt returns SearchUsersUsersUsersPageItemsUser.LinkedAt, and is useful for accessing the field via an interface. +func (v *SearchUsersUsersUsersPageItemsUser) GetLinkedAt() *string { return v.UserFields.LinkedAt } + func (v *SearchUsersUsersUsersPageItemsUser) UnmarshalJSON(b []byte) error { if string(b) == "null" { @@ -11270,6 +11421,16 @@ type __premarshalSearchUsersUsersUsersPageItemsUser struct { GithubUsername *string `json:"githubUsername"` Roles []Role `json:"roles"` + + IdentityProvider *string `json:"identityProvider"` + + GithubId *int `json:"githubId"` + + ExternalId *string `json:"externalId"` + + ExternalAppId *string `json:"externalAppId"` + + LinkedAt *string `json:"linkedAt"` } func (v *SearchUsersUsersUsersPageItemsUser) MarshalJSON() ([]byte, error) { @@ -11289,6 +11450,11 @@ func (v *SearchUsersUsersUsersPageItemsUser) __premarshalJSON() (*__premarshalSe retval.Handle = v.UserFields.Handle retval.GithubUsername = v.UserFields.GithubUsername retval.Roles = v.UserFields.Roles + retval.IdentityProvider = v.UserFields.IdentityProvider + retval.GithubId = v.UserFields.GithubId + retval.ExternalId = v.UserFields.ExternalId + retval.ExternalAppId = v.UserFields.ExternalAppId + retval.LinkedAt = v.UserFields.LinkedAt return &retval, nil } @@ -12906,6 +13072,25 @@ func (v *UpdateMyProfileUpdateMyProfileUser) GetGithubUsername() *string { // GetRoles returns UpdateMyProfileUpdateMyProfileUser.Roles, and is useful for accessing the field via an interface. func (v *UpdateMyProfileUpdateMyProfileUser) GetRoles() []Role { return v.UserFields.Roles } +// GetIdentityProvider returns UpdateMyProfileUpdateMyProfileUser.IdentityProvider, and is useful for accessing the field via an interface. +func (v *UpdateMyProfileUpdateMyProfileUser) GetIdentityProvider() *string { + return v.UserFields.IdentityProvider +} + +// GetGithubId returns UpdateMyProfileUpdateMyProfileUser.GithubId, and is useful for accessing the field via an interface. +func (v *UpdateMyProfileUpdateMyProfileUser) GetGithubId() *int { return v.UserFields.GithubId } + +// GetExternalId returns UpdateMyProfileUpdateMyProfileUser.ExternalId, and is useful for accessing the field via an interface. +func (v *UpdateMyProfileUpdateMyProfileUser) GetExternalId() *string { return v.UserFields.ExternalId } + +// GetExternalAppId returns UpdateMyProfileUpdateMyProfileUser.ExternalAppId, and is useful for accessing the field via an interface. +func (v *UpdateMyProfileUpdateMyProfileUser) GetExternalAppId() *string { + return v.UserFields.ExternalAppId +} + +// GetLinkedAt returns UpdateMyProfileUpdateMyProfileUser.LinkedAt, and is useful for accessing the field via an interface. +func (v *UpdateMyProfileUpdateMyProfileUser) GetLinkedAt() *string { return v.UserFields.LinkedAt } + func (v *UpdateMyProfileUpdateMyProfileUser) UnmarshalJSON(b []byte) error { if string(b) == "null" { @@ -12943,6 +13128,16 @@ type __premarshalUpdateMyProfileUpdateMyProfileUser struct { GithubUsername *string `json:"githubUsername"` Roles []Role `json:"roles"` + + IdentityProvider *string `json:"identityProvider"` + + GithubId *int `json:"githubId"` + + ExternalId *string `json:"externalId"` + + ExternalAppId *string `json:"externalAppId"` + + LinkedAt *string `json:"linkedAt"` } func (v *UpdateMyProfileUpdateMyProfileUser) MarshalJSON() ([]byte, error) { @@ -12962,6 +13157,11 @@ func (v *UpdateMyProfileUpdateMyProfileUser) __premarshalJSON() (*__premarshalUp retval.Handle = v.UserFields.Handle retval.GithubUsername = v.UserFields.GithubUsername retval.Roles = v.UserFields.Roles + retval.IdentityProvider = v.UserFields.IdentityProvider + retval.GithubId = v.UserFields.GithubId + retval.ExternalId = v.UserFields.ExternalId + retval.ExternalAppId = v.UserFields.ExternalAppId + retval.LinkedAt = v.UserFields.LinkedAt return &retval, nil } @@ -13367,6 +13567,25 @@ func (v *UpdateOrgMemberUpdateOrgMemberUser) GetGithubUsername() *string { // GetRoles returns UpdateOrgMemberUpdateOrgMemberUser.Roles, and is useful for accessing the field via an interface. func (v *UpdateOrgMemberUpdateOrgMemberUser) GetRoles() []Role { return v.UserFields.Roles } +// GetIdentityProvider returns UpdateOrgMemberUpdateOrgMemberUser.IdentityProvider, and is useful for accessing the field via an interface. +func (v *UpdateOrgMemberUpdateOrgMemberUser) GetIdentityProvider() *string { + return v.UserFields.IdentityProvider +} + +// GetGithubId returns UpdateOrgMemberUpdateOrgMemberUser.GithubId, and is useful for accessing the field via an interface. +func (v *UpdateOrgMemberUpdateOrgMemberUser) GetGithubId() *int { return v.UserFields.GithubId } + +// GetExternalId returns UpdateOrgMemberUpdateOrgMemberUser.ExternalId, and is useful for accessing the field via an interface. +func (v *UpdateOrgMemberUpdateOrgMemberUser) GetExternalId() *string { return v.UserFields.ExternalId } + +// GetExternalAppId returns UpdateOrgMemberUpdateOrgMemberUser.ExternalAppId, and is useful for accessing the field via an interface. +func (v *UpdateOrgMemberUpdateOrgMemberUser) GetExternalAppId() *string { + return v.UserFields.ExternalAppId +} + +// GetLinkedAt returns UpdateOrgMemberUpdateOrgMemberUser.LinkedAt, and is useful for accessing the field via an interface. +func (v *UpdateOrgMemberUpdateOrgMemberUser) GetLinkedAt() *string { return v.UserFields.LinkedAt } + func (v *UpdateOrgMemberUpdateOrgMemberUser) UnmarshalJSON(b []byte) error { if string(b) == "null" { @@ -13404,6 +13623,16 @@ type __premarshalUpdateOrgMemberUpdateOrgMemberUser struct { GithubUsername *string `json:"githubUsername"` Roles []Role `json:"roles"` + + IdentityProvider *string `json:"identityProvider"` + + GithubId *int `json:"githubId"` + + ExternalId *string `json:"externalId"` + + ExternalAppId *string `json:"externalAppId"` + + LinkedAt *string `json:"linkedAt"` } func (v *UpdateOrgMemberUpdateOrgMemberUser) MarshalJSON() ([]byte, error) { @@ -13423,6 +13652,11 @@ func (v *UpdateOrgMemberUpdateOrgMemberUser) __premarshalJSON() (*__premarshalUp retval.Handle = v.UserFields.Handle retval.GithubUsername = v.UserFields.GithubUsername retval.Roles = v.UserFields.Roles + retval.IdentityProvider = v.UserFields.IdentityProvider + retval.GithubId = v.UserFields.GithubId + retval.ExternalId = v.UserFields.ExternalId + retval.ExternalAppId = v.UserFields.ExternalAppId + retval.LinkedAt = v.UserFields.LinkedAt return &retval, nil } @@ -13558,6 +13792,25 @@ func (v *UpdateUserRolesUpdateUserRolesUser) GetGithubUsername() *string { // GetRoles returns UpdateUserRolesUpdateUserRolesUser.Roles, and is useful for accessing the field via an interface. func (v *UpdateUserRolesUpdateUserRolesUser) GetRoles() []Role { return v.UserFields.Roles } +// GetIdentityProvider returns UpdateUserRolesUpdateUserRolesUser.IdentityProvider, and is useful for accessing the field via an interface. +func (v *UpdateUserRolesUpdateUserRolesUser) GetIdentityProvider() *string { + return v.UserFields.IdentityProvider +} + +// GetGithubId returns UpdateUserRolesUpdateUserRolesUser.GithubId, and is useful for accessing the field via an interface. +func (v *UpdateUserRolesUpdateUserRolesUser) GetGithubId() *int { return v.UserFields.GithubId } + +// GetExternalId returns UpdateUserRolesUpdateUserRolesUser.ExternalId, and is useful for accessing the field via an interface. +func (v *UpdateUserRolesUpdateUserRolesUser) GetExternalId() *string { return v.UserFields.ExternalId } + +// GetExternalAppId returns UpdateUserRolesUpdateUserRolesUser.ExternalAppId, and is useful for accessing the field via an interface. +func (v *UpdateUserRolesUpdateUserRolesUser) GetExternalAppId() *string { + return v.UserFields.ExternalAppId +} + +// GetLinkedAt returns UpdateUserRolesUpdateUserRolesUser.LinkedAt, and is useful for accessing the field via an interface. +func (v *UpdateUserRolesUpdateUserRolesUser) GetLinkedAt() *string { return v.UserFields.LinkedAt } + func (v *UpdateUserRolesUpdateUserRolesUser) UnmarshalJSON(b []byte) error { if string(b) == "null" { @@ -13595,6 +13848,16 @@ type __premarshalUpdateUserRolesUpdateUserRolesUser struct { GithubUsername *string `json:"githubUsername"` Roles []Role `json:"roles"` + + IdentityProvider *string `json:"identityProvider"` + + GithubId *int `json:"githubId"` + + ExternalId *string `json:"externalId"` + + ExternalAppId *string `json:"externalAppId"` + + LinkedAt *string `json:"linkedAt"` } func (v *UpdateUserRolesUpdateUserRolesUser) MarshalJSON() ([]byte, error) { @@ -13614,6 +13877,11 @@ func (v *UpdateUserRolesUpdateUserRolesUser) __premarshalJSON() (*__premarshalUp retval.Handle = v.UserFields.Handle retval.GithubUsername = v.UserFields.GithubUsername retval.Roles = v.UserFields.Roles + retval.IdentityProvider = v.UserFields.IdentityProvider + retval.GithubId = v.UserFields.GithubId + retval.ExternalId = v.UserFields.ExternalId + retval.ExternalAppId = v.UserFields.ExternalAppId + retval.LinkedAt = v.UserFields.LinkedAt return &retval, nil } @@ -13649,14 +13917,24 @@ func (v *UserApiKeyFields) GetLastUsedAt() *string { return v.LastUsedAt } // GetRevokedAt returns UserApiKeyFields.RevokedAt, and is useful for accessing the field via an interface. func (v *UserApiKeyFields) GetRevokedAt() *string { return v.RevokedAt } -// UserFields includes the GraphQL fields of User requested by the fragment UserFields. +// The identity fields (identityProvider/githubId/externalId/externalAppId) +// are load-bearing for duplicate-account triage: `user merge` folds a +// source-only value into an EMPTY target field only, so a populated +// collision is cleared from the source and that login stops working. Seeing +// both sides' providers is what tells a merge apart from a lockout +// (cor:api:010:02). type UserFields struct { - Id string `json:"id"` - Name *string `json:"name"` - Email *string `json:"email"` - Handle *string `json:"handle"` - GithubUsername *string `json:"githubUsername"` - Roles []Role `json:"roles"` + Id string `json:"id"` + Name *string `json:"name"` + Email *string `json:"email"` + Handle *string `json:"handle"` + GithubUsername *string `json:"githubUsername"` + Roles []Role `json:"roles"` + IdentityProvider *string `json:"identityProvider"` + GithubId *int `json:"githubId"` + ExternalId *string `json:"externalId"` + ExternalAppId *string `json:"externalAppId"` + LinkedAt *string `json:"linkedAt"` } // GetId returns UserFields.Id, and is useful for accessing the field via an interface. @@ -13677,6 +13955,21 @@ func (v *UserFields) GetGithubUsername() *string { return v.GithubUsername } // GetRoles returns UserFields.Roles, and is useful for accessing the field via an interface. func (v *UserFields) GetRoles() []Role { return v.Roles } +// GetIdentityProvider returns UserFields.IdentityProvider, and is useful for accessing the field via an interface. +func (v *UserFields) GetIdentityProvider() *string { return v.IdentityProvider } + +// GetGithubId returns UserFields.GithubId, and is useful for accessing the field via an interface. +func (v *UserFields) GetGithubId() *int { return v.GithubId } + +// GetExternalId returns UserFields.ExternalId, and is useful for accessing the field via an interface. +func (v *UserFields) GetExternalId() *string { return v.ExternalId } + +// GetExternalAppId returns UserFields.ExternalAppId, and is useful for accessing the field via an interface. +func (v *UserFields) GetExternalAppId() *string { return v.ExternalAppId } + +// GetLinkedAt returns UserFields.LinkedAt, and is useful for accessing the field via an interface. +func (v *UserFields) GetLinkedAt() *string { return v.LinkedAt } + // __AcceptInvitationInput is used internally by genqlient type __AcceptInvitationInput struct { Slug string `json:"slug"` @@ -15091,13 +15384,13 @@ func (v *__SearchReplaceInNodesInput) GetInput() *SearchReplaceInNodesInput { re // __SearchUsersInput is used internally by genqlient type __SearchUsersInput struct { - Query string `json:"query"` - Limit *int `json:"limit,omitempty"` - Offset *int `json:"offset,omitempty"` + Query *string `json:"query,omitempty"` + Limit *int `json:"limit,omitempty"` + Offset *int `json:"offset,omitempty"` } // GetQuery returns __SearchUsersInput.Query, and is useful for accessing the field via an interface. -func (v *__SearchUsersInput) GetQuery() string { return v.Query } +func (v *__SearchUsersInput) GetQuery() *string { return v.Query } // GetLimit returns __SearchUsersInput.Limit, and is useful for accessing the field via an interface. func (v *__SearchUsersInput) GetLimit() *int { return v.Limit } @@ -15649,6 +15942,11 @@ fragment UserFields on User { handle githubUsername roles + identityProvider + githubId + externalId + externalAppId + linkedAt } ` @@ -18580,6 +18878,11 @@ fragment UserFields on User { handle githubUsername roles + identityProvider + githubId + externalId + externalAppId + linkedAt } ` @@ -19220,6 +19523,11 @@ fragment UserFields on User { handle githubUsername roles + identityProvider + githubId + externalId + externalAppId + linkedAt } ` @@ -19700,6 +20008,11 @@ fragment UserFields on User { handle githubUsername roles + identityProvider + githubId + externalId + externalAppId + linkedAt } ` @@ -20494,7 +20807,7 @@ func SearchReplaceInNodes( // The query executed by SearchUsers. const SearchUsers_Operation = ` -query SearchUsers ($query: String!, $limit: Int, $offset: Int) { +query SearchUsers ($query: String, $limit: Int, $offset: Int) { users(filter: {query:$query}, limit: $limit, offset: $offset) { total items { @@ -20509,6 +20822,11 @@ fragment UserFields on User { handle githubUsername roles + identityProvider + githubId + externalId + externalAppId + linkedAt } ` @@ -20517,10 +20835,16 @@ fragment UserFields on User { // handle/githubUsername substring, email exact). Results are name-ascending, // so an EXACT identifier match can sit past the first 200-cap page — user-ref // resolution pages via api.CollectUntil with an exact-match short-circuit. +// +// $query is nullable and omitempty: a nil pointer drops the variable, which +// GraphQL input-object coercion renders as `filter: {}` — i.e. an omitted +// query. That is the platform ADMIN/OWNER full-user-list path (a non-admin +// gets an empty page instead, per cor:acl:070:02), and it is the only way to +// enumerate accounts nobody thought to search for. func SearchUsers( ctx_ context.Context, client_ graphql.Client, - query string, + query *string, limit *int, offset *int, ) (data_ *SearchUsersResponse, err_ error) { @@ -21237,6 +21561,11 @@ fragment UserFields on User { handle githubUsername roles + identityProvider + githubId + externalId + externalAppId + linkedAt } ` @@ -21480,6 +21809,11 @@ fragment UserFields on User { handle githubUsername roles + identityProvider + githubId + externalId + externalAppId + linkedAt } ` @@ -21574,6 +21908,11 @@ fragment UserFields on User { handle githubUsername roles + identityProvider + githubId + externalId + externalAppId + linkedAt } ` diff --git a/internal/api/queries/org.graphql b/internal/api/queries/org.graphql index 1e6f6d7..3033b0b 100644 --- a/internal/api/queries/org.graphql +++ b/internal/api/queries/org.graphql @@ -10,6 +10,12 @@ fragment OrgFields on Organization { updatedAt } +# The identity fields (identityProvider/githubId/externalId/externalAppId) +# are load-bearing for duplicate-account triage: `user merge` folds a +# source-only value into an EMPTY target field only, so a populated +# collision is cleared from the source and that login stops working. Seeing +# both sides' providers is what tells a merge apart from a lockout +# (cor:api:010:02). fragment UserFields on User { id name @@ -17,6 +23,11 @@ fragment UserFields on User { handle githubUsername roles + identityProvider + githubId + externalId + externalAppId + linkedAt } # organization(ref:) accepts the org's PK id or URN (hadron-server#473). @@ -56,8 +67,15 @@ query GetUser($ref: ID!) { # handle/githubUsername substring, email exact). Results are name-ascending, # so an EXACT identifier match can sit past the first 200-cap page — user-ref # resolution pages via api.CollectUntil with an exact-match short-circuit. +# +# $query is nullable and omitempty: a nil pointer drops the variable, which +# GraphQL input-object coercion renders as `filter: {}` — i.e. an omitted +# query. That is the platform ADMIN/OWNER full-user-list path (a non-admin +# gets an empty page instead, per cor:acl:070:02), and it is the only way to +# enumerate accounts nobody thought to search for. query SearchUsers( - $query: String! + # @genqlient(omitempty: true) + $query: String # @genqlient(omitempty: true) $limit: Int # @genqlient(omitempty: true) diff --git a/internal/cmd/agentic/agentic-usage.md b/internal/cmd/agentic/agentic-usage.md index a1c8664..957cf70 100644 --- a/internal/cmd/agentic/agentic-usage.md +++ b/internal/cmd/agentic/agentic-usage.md @@ -79,7 +79,7 @@ hadron app list --org | install (--org | --owner-me) --agent -- hadron ai-config list [--app ] [--agent ] | create (--app|--agent|--org ) --name --provider

--model [--api-key -] [--file ] | update ... | rm hadron org list [--mine] | create --name --urn | get | public | update | rm | member list|add|set-role|rm --user [--role ] | invite create --org --role | invite accept | invite show hadron agent list [--org ] [--type ASSISTANT|CHATBOT] [--visibility ORGANIZATION|PERSONAL|PUBLIC] | list --public [--type ] [--limit N] [--offset N] | get | create --name [--org | --owner-me] [--type ] [--visibility ] [--description ] [--system-prompt

] [--system-memory ] [--surface ]… | update [] | rm --yes -hadron user search [--limit N] [--offset N] | set-roles --role ... --yes | merge --into --yes +hadron user search [query] [--limit N] [--offset N] | set-roles --role ... --yes | merge --into --yes hadron profile set [--name ] [--email ] [--handle ] hadron server-info hadron run trigger --app --entry [--as-self] [--arg k=v]... [--ai-config ] [--wait] | list [--app | --org ] [--status ] | get | cancel --yes @@ -245,7 +245,13 @@ Conventions: `hrn:user:` URN, passed through verbatim (the server resolves and authorizes — platform ADMIN/OWNER, or an org ADMIN/OWNER over both members). No server dry-run; the confirmation is the last local safety boundary, so it's gated - — pass `--yes` non-interactively. + — pass `--yes` non-interactively. Data is safe (memberships, owned memories, + grants and connections transfer atomically, and role collisions keep the + strongest **live** entitlement), but **logins are not**: a source field fills the + target only where the target's is empty, so a populated collision is cleared from + the source and that credential stops authenticating. Compare both accounts' + `identityProvider`/`email`/`githubId` via `user search` first, and pick the + target for the handle you want to keep — there is no admin handle rename. - `server-info` reports the hadron-server this invocation targets: `{url, version, baseUrl, authenticated}`. It works **signed out** (the field is public), so it doubles as a reachability probe — a failure REACHING the @@ -558,8 +564,16 @@ Conventions: requires `--yes`. Memory-attach, AI-config wiring, and app-wiring land next. - `user search ` finds users (enumeration-safe: substring on handle / GitHub username, exact on email) — the way to resolve a user ID for `org - member`/`memory member`/`memory share`. `profile set [--name|--email|--handle]` - updates YOUR own profile; only the fields you pass change. + member`/`memory member`/`memory share`. **Omit the query** (also spelled `user + list`) to list every user, the unfiltered listing a platform ADMIN/OWNER may + run; a caller without platform authority gets an empty list, not an error. + Without an explicit `--limit`/`--offset` it pages to exhaustion, so the + population is never truncated at one server page; either flag selects a single + explicit page. Each user carries `identityProvider`, `githubId`, `externalId`, + `externalAppId`, and `linkedAt` alongside the profile fields — read those before + `user merge`, since a login only survives a merge if the target's corresponding + field is empty. `profile set [--name|--email|--handle]` updates YOUR own + profile; only the fields you pass change. - `access check ` answers "what access does this user have to this resource?" — the authoritative, server-computed effective access plus the grants that confer it (no client-side re-derivation). `` is an id, email, diff --git a/internal/cmd/list_naming_test.go b/internal/cmd/list_naming_test.go index eb286fd..697e9e8 100644 --- a/internal/cmd/list_naming_test.go +++ b/internal/cmd/list_naming_test.go @@ -138,6 +138,7 @@ func TestBothSpellingsResolve(t *testing.T) { {{"edge", "ls"}, {"edge", "list"}}, {{"auth", "token", "ls"}, {"auth", "token", "list"}}, {{"object", "ls"}, {"object", "list"}}, + {{"user", "ls"}, {"user", "list"}}, } root := NewRootCmd(cmdutil.NewFactory()) for _, pair := range paths { diff --git a/internal/cmd/user/user.go b/internal/cmd/user/user.go index d8ac8a7..f6d9522 100644 --- a/internal/cmd/user/user.go +++ b/internal/cmd/user/user.go @@ -17,14 +17,22 @@ import ( "github.com/hadron-memory/hadron-cli/internal/output" ) -// userDTO is the stable --json shape for a user. +// userDTO is the stable --json shape for a user. The identity fields are +// additive (#325): they let duplicate-account triage see which login each +// account actually authenticates with before `user merge` clears a +// colliding one — see cor:api:010:02. type userDTO struct { - ID string `json:"id"` - Name *string `json:"name"` - Email *string `json:"email"` - Handle *string `json:"handle"` - GithubUsername *string `json:"githubUsername"` - Roles []string `json:"roles"` + ID string `json:"id"` + Name *string `json:"name"` + Email *string `json:"email"` + Handle *string `json:"handle"` + GithubUsername *string `json:"githubUsername"` + Roles []string `json:"roles"` + IdentityProvider *string `json:"identityProvider"` + GithubID *int `json:"githubId"` + ExternalID *string `json:"externalId"` + ExternalAppID *string `json:"externalAppId"` + LinkedAt *string `json:"linkedAt"` } func userDTOFromFields(u gen.UserFields) userDTO { @@ -32,7 +40,19 @@ func userDTOFromFields(u gen.UserFields) userDTO { for _, r := range u.Roles { roles = append(roles, string(r)) } - return userDTO{ID: u.Id, Name: u.Name, Email: u.Email, Handle: u.Handle, GithubUsername: u.GithubUsername, Roles: roles} + return userDTO{ + ID: u.Id, + Name: u.Name, + Email: u.Email, + Handle: u.Handle, + GithubUsername: u.GithubUsername, + Roles: roles, + IdentityProvider: u.IdentityProvider, + GithubID: u.GithubId, + ExternalID: u.ExternalId, + ExternalAppID: u.ExternalAppId, + LinkedAt: u.LinkedAt, + } } func dash(s *string) string { @@ -142,15 +162,32 @@ checks. Spec cor:api:010:02.`, func newCmdSearch(f *cmdutil.Factory) *cobra.Command { var limit, offset int cmd := &cobra.Command{ - Use: "search ", - Short: "Search users by handle, GitHub username, or exact email", + Use: "search [query]", + Aliases: []string{"ls", "list"}, + Short: "Search users, or list them all as a platform admin", Long: `Search users. Matching is enumeration-safe: substring on handle and -GitHub username, exact on email. Results are name-ascending.`, - Example: ` hadron user search alice --json`, - Args: cobra.ExactArgs(1), +GitHub username, exact on email. Results are name-ascending. + +Omit the query to list every user — the unfiltered listing a platform +ADMIN/OWNER may run (cor:acl:070:02). A caller without platform authority gets +an empty list rather than an error, since the server will not confirm that +accounts it won't show you exist. + +Without an explicit --limit/--offset the listing pages to exhaustion, so a +population larger than one 200-row server page is never silently truncated. +Passing either flag selects a single explicit page instead.`, + Example: ` hadron user search alice --json + hadron user search --json # every user (platform admin)`, + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if strings.TrimSpace(args[0]) == "" { - return exitcode.Newf(exitcode.Usage, "query must not be empty") + // An absent query means "list all"; an explicitly empty one is a + // typo, not a request to enumerate the platform. + var query *string + if len(args) == 1 { + if strings.TrimSpace(args[0]) == "" { + return exitcode.Newf(exitcode.Usage, "query must not be empty — omit it entirely to list all users") + } + query = &args[0] } if limit < 0 { return exitcode.Newf(exitcode.Usage, "limit must be non-negative") @@ -162,37 +199,65 @@ GitHub username, exact on email. Results are name-ascending.`, if err != nil { return err } - var lim, off *int - if cmd.Flags().Changed("limit") { - lim = &limit + + fetch := func(lim, off *int) ([]*gen.SearchUsersUsersUsersPageItemsUser, int, error) { + resp, err := gen.SearchUsers(cmd.Context(), client, query, lim, off) + if err != nil { + return nil, 0, api.MapError(err) + } + if resp == nil || resp.Users == nil { + return nil, 0, nil + } + return resp.Users.Items, resp.Users.Total, nil } - if cmd.Flags().Changed("offset") { - off = &offset + + var items []*gen.SearchUsersUsersUsersPageItemsUser + paged := cmd.Flags().Changed("limit") || cmd.Flags().Changed("offset") + if paged { + var lim, off *int + if cmd.Flags().Changed("limit") { + lim = &limit + } + if cmd.Flags().Changed("offset") { + off = &offset + } + items, _, err = fetch(lim, off) + } else { + items, err = api.CollectAll(func(lim, off int) ([]*gen.SearchUsersUsersUsersPageItemsUser, int, error) { + return fetch(&lim, &off) + }) } - resp, err := gen.SearchUsers(cmd.Context(), client, args[0], lim, off) if err != nil { - return api.MapError(err) + return err } + users := []userDTO{} - if resp.Users != nil { - for _, u := range resp.Users.Items { - if u == nil { - continue - } - users = append(users, userDTOFromFields(u.UserFields)) + for _, u := range items { + if u == nil { + continue } + users = append(users, userDTOFromFields(u.UserFields)) } return output.Write(f.IOStreams, f.JSON, users, func(w io.Writer) error { - t := output.NewTable(w, "ID", "NAME", "EMAIL", "HANDLE", "GITHUB") + // PROVIDER earns a column because it is the field that decides + // whether merging two accounts preserves or destroys a login. + t := output.NewTable(w, "ID", "NAME", "EMAIL", "HANDLE", "GITHUB", "PROVIDER") for _, u := range users { - t.Row(u.ID, dash(u.Name), dash(u.Email), dash(u.Handle), dash(u.GithubUsername)) + t.Row(u.ID, dash(u.Name), dash(u.Email), dash(u.Handle), dash(u.GithubUsername), dash(u.IdentityProvider)) + } + if err := t.Flush(); err != nil { + return err + } + if query == nil && len(users) == 0 { + _, err := fmt.Fprintln(w, "\nNo users listed. The unfiltered listing requires platform ADMIN/OWNER; pass a query to search instead.") + return err } - return t.Flush() + return nil }) }, } - cmd.Flags().IntVar(&limit, "limit", 0, "max results (server default when unset)") - cmd.Flags().IntVar(&offset, "offset", 0, "results to skip") + cmd.Flags().IntVar(&limit, "limit", 0, "max results for a single explicit page (default: page to exhaustion)") + cmd.Flags().IntVar(&offset, "offset", 0, "results to skip (selects a single explicit page)") return cmd } diff --git a/internal/cmd/user_cmd_test.go b/internal/cmd/user_cmd_test.go index 2b03282..6e9b507 100644 --- a/internal/cmd/user_cmd_test.go +++ b/internal/cmd/user_cmd_test.go @@ -2,6 +2,8 @@ package cmd import ( "encoding/json" + "net/http" + "net/http/httptest" "strings" "testing" @@ -38,6 +40,154 @@ func TestUserSearch(t *testing.T) { } } +// uIdentityUserJSON is a GitHub-login account with no email — the shape whose +// identity fields decide whether merging it into an email account preserves +// the GitHub login or destroys it (cor:api:010:02). +const uIdentityUserJSON = `{"id":"usr2","name":"Bob","email":null,"handle":"bob", + "githubUsername":"bobgh","roles":["READER"],"identityProvider":"github","githubId":4242, + "externalId":"gh|4242","externalAppId":null,"linkedAt":"2026-07-01T00:00:00Z"}` + +func TestUserSearchSurfacesIdentityFields(t *testing.T) { + gql, _ := captureGraphQL(t, map[string]string{ + "SearchUsers": `{"data":{"users":{"total":1,"items":[` + uIdentityUserJSON + `]}}}`, + }) + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"user", "search", "bob", "--json", "--server", gql.URL}) + if err := root.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + var users []struct { + IdentityProvider *string `json:"identityProvider"` + GithubID *int `json:"githubId"` + ExternalID *string `json:"externalId"` + LinkedAt *string `json:"linkedAt"` + } + if err := json.Unmarshal([]byte(out.String()), &users); err != nil { + t.Fatalf("not a JSON array: %v\n%s", err, out.String()) + } + if len(users) != 1 { + t.Fatalf("users: %+v", users) + } + u := users[0] + if u.IdentityProvider == nil || *u.IdentityProvider != "github" { + t.Errorf("identityProvider: %v", u.IdentityProvider) + } + if u.GithubID == nil || *u.GithubID != 4242 { + t.Errorf("githubId: %v", u.GithubID) + } + if u.ExternalID == nil || *u.ExternalID != "gh|4242" { + t.Errorf("externalId: %v", u.ExternalID) + } + if u.LinkedAt == nil || *u.LinkedAt != "2026-07-01T00:00:00Z" { + t.Errorf("linkedAt: %v", u.LinkedAt) + } +} + +// No positional arg must OMIT the query variable entirely — `filter: {}` is +// what the server reads as the platform-admin full-list request. Sending an +// explicit empty string would instead be a blank query (an empty page). +func TestUserSearchOmitsQueryVariableForFullList(t *testing.T) { + gql, captured := captureGraphQL(t, map[string]string{ + "SearchUsers": `{"data":{"users":{"total":1,"items":[` + uUserJSON + `]}}}`, + }) + f, _ := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"user", "search", "--json", "--server", gql.URL}) + if err := root.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + var vars map[string]any + _ = json.Unmarshal(captured["SearchUsers"], &vars) + if _, present := vars["query"]; present { + t.Errorf("query variable must be omitted for the full list, got %v", vars) + } +} + +// A non-admin gets an empty page rather than an error; say why instead of +// rendering a bare empty table. +func TestUserSearchFullListHintsWhenEmpty(t *testing.T) { + gql, _ := captureGraphQL(t, map[string]string{ + "SearchUsers": `{"data":{"users":{"total":0,"items":[]}}}`, + }) + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"user", "search", "--server", gql.URL}) + if err := root.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + if !strings.Contains(out.String(), "platform ADMIN/OWNER") { + t.Errorf("expected a permission hint, got:\n%s", out.String()) + } +} + +// searchPagesServer serves a 3-user population two rows at a time, recording +// the offset of every request. +func searchPagesServer(t *testing.T, offsets *[]int) *httptest.Server { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body struct { + Variables struct { + Offset *int `json:"offset"` + } `json:"variables"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + off := 0 + if body.Variables.Offset != nil { + off = *body.Variables.Offset + } + *offsets = append(*offsets, off) + items := `{"id":"u3","handle":"c","roles":[]}` + if off == 0 { + items = `{"id":"u1","handle":"a","roles":[]},{"id":"u2","handle":"b","roles":[]}` + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"data":{"users":{"total":3,"items":[` + items + `]}}}`)) + })) + t.Cleanup(server.Close) + return server +} + +// The whole-population contract: a listing that spans pages must drain, or +// duplicate detection quietly misses the tail (the issue-#23 failure mode). +func TestUserSearchPagesToExhaustion(t *testing.T) { + var offsets []int + gql := searchPagesServer(t, &offsets) + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"user", "search", "--json", "--server", gql.URL}) + if err := root.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + var users []struct { + ID string `json:"id"` + } + if err := json.Unmarshal([]byte(out.String()), &users); err != nil { + t.Fatalf("not a JSON array: %v\n%s", err, out.String()) + } + if len(users) != 3 { + t.Errorf("want all 3 users, got %d: %+v", len(users), users) + } + if len(offsets) != 2 || offsets[0] != 0 || offsets[1] != 2 { + t.Errorf("want offsets [0 2], got %v", offsets) + } +} + +// An explicit --limit is a request for one page, not a drain. +func TestUserSearchExplicitLimitIsSinglePage(t *testing.T) { + var offsets []int + gql := searchPagesServer(t, &offsets) + f, _ := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"user", "search", "--limit", "2", "--json", "--server", gql.URL}) + if err := root.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + if len(offsets) != 1 { + t.Errorf("want exactly one request with an explicit --limit, got %v", offsets) + } +} + func TestProfileSet(t *testing.T) { gql, captured := captureGraphQL(t, map[string]string{ "UpdateMyProfile": `{"data":{"updateMyProfile":` + uUserJSON + `}}`, diff --git a/internal/cmdutil/userref.go b/internal/cmdutil/userref.go index da545ee..dbfc8f8 100644 --- a/internal/cmdutil/userref.go +++ b/internal/cmdutil/userref.go @@ -76,7 +76,9 @@ func ResolveUser(cmd *cobra.Command, client graphql.Client, ref string) (fields // fuzzy-ambiguous. items, err := api.CollectUntil( func(limit, offset int) ([]*gen.SearchUsersUsersUsersPageItemsUser, int, error) { - resp, err := gen.SearchUsers(cmd.Context(), client, token, &limit, &offset) + // Always a concrete query here — ref resolution never wants the + // unfiltered admin list. + resp, err := gen.SearchUsers(cmd.Context(), client, &token, &limit, &offset) if err != nil { return nil, 0, api.MapError(err) } From 3a0a5f4dd98c4d51b65e445c06b07ba5310a61a4 Mon Sep 17 00:00:00 2001 From: Holger Selover-Stephan Date: Thu, 30 Jul 2026 17:31:22 +0200 Subject: [PATCH 2/2] Address review feedback: surface identity fields in org member list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex was right that the plan doc claimed more than the code did: adding fields to the shared UserFields fragment only makes them available, since every command marshals its own DTO to keep --json stable. `org member list` has its own userDTO and was dropping them on the floor — extended it to match, with a test, because a member roster is where two rows for the same human are most visible. `access check` selects UserFields only to resolve an id and emits no user object, so the doc no longer claims it surfaces anything. Copilot correctly caught a stale comment on the PROVIDER column asserting identityProvider "decides whether merging preserves or destroys a login" — that is the misreading this PR exists to correct. It explains an account's origin; auth resolves by provider id then email. Also tightened two tests that swallowed a decode error: the full-list assertion is a negative one and would have passed vacuously on an empty map, and a decode failure in the paging fake would have looked like offset=0. Co-Authored-By: Claude Opus 5 --- docs/plans/user-list-identity-fields.md | 12 +++++--- internal/cmd/org/common.go | 37 +++++++++++++++-------- internal/cmd/org_cmd_test.go | 39 +++++++++++++++++++++++++ internal/cmd/user/user.go | 9 ++++-- internal/cmd/user_cmd_test.go | 16 ++++++++-- 5 files changed, 93 insertions(+), 20 deletions(-) diff --git a/docs/plans/user-list-identity-fields.md b/docs/plans/user-list-identity-fields.md index fc572c1..19d4964 100644 --- a/docs/plans/user-list-identity-fields.md +++ b/docs/plans/user-list-identity-fields.md @@ -50,10 +50,14 @@ returned an empty page and looked like a permissions problem. The command still rejects an explicitly empty argument as usage, since `user list ""` is a typo, not a request to enumerate the platform. -**The identity fields go on the shared `UserFields` fragment**, so `org member -list` and `access check` surface them too. They are the fields that distinguish -"two rows for one human" from "two humans", and that judgement is made wherever -users are listed, not only in `user list`. +**The identity fields go on the shared `UserFields` fragment**, but that alone +surfaces nothing: every command marshals its own DTO so `--json` stays stable +across regenerations, so the fragment only makes the data *available*. `org +member list` has its own `userDTO` in `internal/cmd/org/common.go` and was +extended to match — a roster of one org's members is where two rows for the same +human are most visible, and that judgement is made wherever users are listed, +not only in `user list`. `access check` also selects `UserFields`, but only to +resolve an id; it emits no user object, so there is nothing to surface there. **`identityProvider` is displayed but is not the answer.** It is tempting to read it as the login method; it is not. Auth resolves a user by *provider id* diff --git a/internal/cmd/org/common.go b/internal/cmd/org/common.go index 1b1d66f..e94c238 100644 --- a/internal/cmd/org/common.go +++ b/internal/cmd/org/common.go @@ -29,13 +29,21 @@ type orgDTO struct { } // userDTO is the stable --json shape for a user (search results, member.user). +// The identity fields mirror the user package's DTO: a roster of one org's +// members is where two rows for the same human are most visible, and telling +// that apart from two humans needs the provider identity, not just the profile. type userDTO struct { - ID string `json:"id"` - Name *string `json:"name"` - Email *string `json:"email"` - Handle *string `json:"handle"` - GithubUsername *string `json:"githubUsername"` - Roles []string `json:"roles"` + ID string `json:"id"` + Name *string `json:"name"` + Email *string `json:"email"` + Handle *string `json:"handle"` + GithubUsername *string `json:"githubUsername"` + Roles []string `json:"roles"` + IdentityProvider *string `json:"identityProvider"` + GithubID *int `json:"githubId"` + ExternalID *string `json:"externalId"` + ExternalAppID *string `json:"externalAppId"` + LinkedAt *string `json:"linkedAt"` } // memberDTO is the stable --json shape for an org membership. CanInvite is only @@ -66,12 +74,17 @@ func userDTOFromFields(u gen.UserFields) userDTO { roles = append(roles, string(r)) } return userDTO{ - ID: u.Id, - Name: u.Name, - Email: u.Email, - Handle: u.Handle, - GithubUsername: u.GithubUsername, - Roles: roles, + ID: u.Id, + Name: u.Name, + Email: u.Email, + Handle: u.Handle, + GithubUsername: u.GithubUsername, + Roles: roles, + IdentityProvider: u.IdentityProvider, + GithubID: u.GithubId, + ExternalID: u.ExternalId, + ExternalAppID: u.ExternalAppId, + LinkedAt: u.LinkedAt, } } diff --git a/internal/cmd/org_cmd_test.go b/internal/cmd/org_cmd_test.go index 514486a..9ee2bd5 100644 --- a/internal/cmd/org_cmd_test.go +++ b/internal/cmd/org_cmd_test.go @@ -291,6 +291,45 @@ func TestOrgMemberLs(t *testing.T) { } } +// A member roster is where duplicate accounts are most visible, so the +// identity fields have to reach org's own DTO — adding them to the shared +// UserFields fragment alone would fetch them and then drop them. +func TestOrgMemberLsSurfacesIdentityFields(t *testing.T) { + member := `{"id":"usr9","name":"Dup","email":null,"handle":"dup","githubUsername":"dupgh", + "roles":["READER"],"identityProvider":"GITHUB","githubId":991,"externalId":null, + "externalAppId":null,"linkedAt":null}` + gql, _ := captureGraphQL(t, map[string]string{ + "OrgMembers": `{"data":{"organization":{"id":"org1","members":[ + {"id":"m1","role":"ADMIN","canInvite":false,"user":` + member + `} + ]}}}`, + }) + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"org", "member", "ls", "org1", "--json", "--server", gql.URL}) + if err := root.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + var members []struct { + User struct { + IdentityProvider *string `json:"identityProvider"` + GithubID *int `json:"githubId"` + } `json:"user"` + } + if err := json.Unmarshal([]byte(out.String()), &members); err != nil { + t.Fatalf("not a JSON array: %v\n%s", err, out.String()) + } + if len(members) != 1 { + t.Fatalf("members: %+v", members) + } + u := members[0].User + if u.IdentityProvider == nil || *u.IdentityProvider != "GITHUB" { + t.Errorf("identityProvider: %v", u.IdentityProvider) + } + if u.GithubID == nil || *u.GithubID != 991 { + t.Errorf("githubId: %v", u.GithubID) + } +} + func TestOrgMemberAdd(t *testing.T) { gql, captured := captureGraphQL(t, map[string]string{ "AddOrgMember": `{"data":{"addOrgMember":{"id":"m1","role":"CONTRIBUTOR","user":` + orgUserJSON + `}}}`, diff --git a/internal/cmd/user/user.go b/internal/cmd/user/user.go index f6d9522..4fbe79a 100644 --- a/internal/cmd/user/user.go +++ b/internal/cmd/user/user.go @@ -239,8 +239,13 @@ Passing either flag selects a single explicit page instead.`, users = append(users, userDTOFromFields(u.UserFields)) } return output.Write(f.IOStreams, f.JSON, users, func(w io.Writer) error { - // PROVIDER earns a column because it is the field that decides - // whether merging two accounts preserves or destroys a login. + // PROVIDER earns a column because it explains an account's + // origin — the cheapest signal that two rows are one human who + // signed in two ways. It does NOT decide what a merge does to a + // login: auth resolves by provider id then email, and + // identityProvider is only the provider that created the row + // (the linking path keeps the original value). The fields that + // decide that are githubId/email and the --json-only ids. t := output.NewTable(w, "ID", "NAME", "EMAIL", "HANDLE", "GITHUB", "PROVIDER") for _, u := range users { t.Row(u.ID, dash(u.Name), dash(u.Email), dash(u.Handle), dash(u.GithubUsername), dash(u.IdentityProvider)) diff --git a/internal/cmd/user_cmd_test.go b/internal/cmd/user_cmd_test.go index 6e9b507..1b221f1 100644 --- a/internal/cmd/user_cmd_test.go +++ b/internal/cmd/user_cmd_test.go @@ -98,7 +98,14 @@ func TestUserSearchOmitsQueryVariableForFullList(t *testing.T) { t.Fatalf("execute: %v", err) } var vars map[string]any - _ = json.Unmarshal(captured["SearchUsers"], &vars) + if err := json.Unmarshal(captured["SearchUsers"], &vars); err != nil { + t.Fatalf("decoding captured variables: %v", err) + } + // This assertion is a NEGATIVE one, so it would pass vacuously on an + // empty map — check something was actually captured first. + if len(vars) == 0 { + t.Fatal("no SearchUsers variables captured") + } if _, present := vars["query"]; present { t.Errorf("query variable must be omitted for the full list, got %v", vars) } @@ -131,7 +138,12 @@ func searchPagesServer(t *testing.T, offsets *[]int) *httptest.Server { Offset *int `json:"offset"` } `json:"variables"` } - _ = json.NewDecoder(r.Body).Decode(&body) + // A decode failure would silently look like offset=0 and make the + // paging assertions pass for the wrong reason. t.Errorf (not Fatalf) + // — this runs on the server's goroutine. + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Errorf("decoding request body: %v", err) + } off := 0 if body.Variables.Offset != nil { off = *body.Variables.Offset