diff --git a/docs/plans/app-member-956.md b/docs/plans/app-member-956.md new file mode 100644 index 00000000..50a6edb7 --- /dev/null +++ b/docs/plans/app-member-956.md @@ -0,0 +1,45 @@ +# `hadron app member add|remove|list` (cli#956) — design as built + +Built from the CHECKED gate on cli#956 (T2 + scoped §10) and the Opus plan on the same issue +(written against hadron-cli main `9e5d14e3`; main had not changed any file it cites at pickup). +Spec: `cor:dmo:050:04` (AppMember), `cor:dmo:050:12` (who manages an App). + +## Surface + +| Command | Wraps | Notes | +| --- | --- | --- | +| `app member add --user --role ` | `ensureAppMember` | idempotent upsert; role required, no default | +| `app member remove --user ` | `removeAppMember` | server answers `true` whether or not a row existed | +| `app member list` | `App.members` | visible rows only | + +The App is the root `--app` flag, else the active App. No positional App. + +## Decisions + +- **`--app ""` refuses.** `Factory.App()` treats a blank flag like an absent one; `memberTargetApp` + checks `Flag("app").Changed` first. The resolved ref (and which branch supplied it) is frozen once and + used for the prompt, the mutation and the receipt. +- **`--user` is passed through verbatim** (id / email / handle / `hrn:user:`): no trim, no case fold, + no pre-resolution, no `list` call to "detect" a role change. Consequently every add and remove is + confirmed; `--yes` skips, non-TTY without it exits 2 with zero requests. +- **Remove uses `Confirm`, not `ConfirmDeletion`** (review:confirm-prompt-tells-the-truth): the change is + reversible, so "cannot be undone" would be false. *Deviation from the plan, which named ConfirmDeletion.* +- **Definite vs unknown outcome** (`api.AppMemberWriteRefused`). The server re-reads the row after + commit, so a masked INTERNAL error can follow a committed upsert. Only `InvalidRoleError`, + `MaxMembersExceededError`, `OrphanAppError`, NOT_FOUND, UNAUTHENTICATED, BAD_USER_INPUT, FORBIDDEN or a + `Forbidden:` message are definite. Anything else (transport, `data`+`errors`, empty payload) exits 7 + "outcome unknown", prints no DTO, is never retried, and points at `app member list`. +- **Exit codes** for the typed refusals were added to `codeForExtension`: `InvalidRoleError` 2, + `MaxMembersExceededError`/`OrphanAppError` 5. +- **Remove reports `status: "absent"`**, never "removed". A non-manager's refusal for any foreign target + (including an unknown one) is printed as the server wrote it — no "user not found" translation. +- **List** selects `role createdAt user { id handle urn }` only (name/email are withheld by the server). + Empty means "no members visible to you"; `app: null` is NotFound; a null row is an error. + +## Residual (not in this cut) + +R113: `callerManagesAppMembers` has no worker-session refusal and `internal/cmd/app` sends no session +header, so a bound worktree's `app member add` acts with its holder's authority (as `app key` does today). +Server convergence onto `managesApp` and a CLI follow-up attaching the session header are separate issues. +The server also has no last-owner guard on role changes/removal; the confirm-every-write rule is the +CLI's only mitigation. diff --git a/internal/api/errors.go b/internal/api/errors.go index 0e1e8955..b304937a 100644 --- a/internal/api/errors.go +++ b/internal/api/errors.go @@ -768,6 +768,13 @@ func codeForExtension(code string) int { return exitcode.Usage case code == "TICKER_NAME_TAKEN" || code == "TICKER_LIMIT_REACHED": return exitcode.Conflict + // App member writes (cli#956, cor:dmo:050:04): a role the primary Agent's + // policy does not list is an argument the caller fixes; the member cap and + // the last-owner guard hold until the App's membership changes. + case code == "InvalidRoleError": + return exitcode.Usage + case code == "MaxMembersExceededError" || code == "OrphanAppError": + return exitcode.Conflict case code == "UNAUTHENTICATED": return exitcode.AuthRequired case code == "NOT_FOUND" || strings.HasSuffix(code, "_NOT_FOUND") || code == "AiServiceConfigNotFoundError": @@ -1222,3 +1229,42 @@ func IsUnknownGraphQLArgument(err error, argument, field string) bool { } return true } + +// AppMemberWriteRefused reports whether err is a DEFINITE refusal of an +// ensureAppMember / removeAppMember call: the server threw before the write, +// inside the authorized transaction, so nothing changed. Anything else — +// transport failures, a data+errors partial, a masked INTERNAL_SERVER_ERROR — +// is outcome unknown, because ensureAppMember re-reads the row AFTER commit and +// a failure there is reported although the upsert committed (#956). +// +// The typed refusals are InvalidRoleError, MaxMembersExceededError and +// OrphanAppError; the server's `Forbidden: …` arrives as plain text under +// INTERNAL_SERVER_ERROR, so it is recognised by its message prefix. EVERY error +// in the list must qualify; one unrecognized entry keeps the answer ambiguous. +// +// An error is a root refusal only when its path names no field below the +// mutation itself: a nested-field error (path ["ensureAppMember","user",…]) means +// the resolver RAN and returned a row, so the write may have committed whatever +// code the nested error carries. Callers must also treat a populated result next +// to an error as unknown (see memberWriteError) — this function sees only errors. +// Like EndRefusedBeforeCommit, call it BEFORE MapError wraps. +func AppMemberWriteRefused(err error) bool { + list := graphQLErrors(err) + if len(list) == 0 { + return false + } + for _, e := range list { + if e == nil || len(e.Path) > 1 { + return false + } + switch extensionCode(e) { + case "InvalidRoleError", "MaxMembersExceededError", "OrphanAppError", + "NOT_FOUND", "UNAUTHENTICATED", "BAD_USER_INPUT", "FORBIDDEN": + continue + } + if !strings.HasPrefix(strings.TrimSpace(e.Message), "Forbidden:") { + return false + } + } + return true +} diff --git a/internal/api/errors_test.go b/internal/api/errors_test.go index 9427c728..9688a2f0 100644 --- a/internal/api/errors_test.go +++ b/internal/api/errors_test.go @@ -66,6 +66,9 @@ func TestMapError(t *testing.T) { {"ticker quiet hours", gqlErr("TICKER_QUIET_HOURS_INVALID"), exitcode.Usage}, {"ticker name", gqlErr("TICKER_NAME_TAKEN"), exitcode.Conflict}, {"ticker cap", gqlErr("TICKER_LIMIT_REACHED"), exitcode.Conflict}, + {"app member invalid role", gqlErr("InvalidRoleError"), exitcode.Usage}, + {"app member cap", gqlErr("MaxMembersExceededError"), exitcode.Conflict}, + {"app member orphan", gqlErr("OrphanAppError"), exitcode.Conflict}, {"reports to self", gqlErr("WORKER_REPORTS_TO_SELF"), exitcode.Usage}, // Host poller and launch refusals (cli#831). {"host key required", gqlErr("HOST_KEY_REQUIRED"), exitcode.Forbidden}, diff --git a/internal/api/gen/generated.go b/internal/api/gen/generated.go index d887d756..9bfff62e 100644 --- a/internal/api/gen/generated.go +++ b/internal/api/gen/generated.go @@ -7094,6 +7094,76 @@ type AppKeysMetadataResponse struct { // GetAppKeys returns AppKeysMetadataResponse.AppKeys, and is useful for accessing the field via an interface. func (v *AppKeysMetadataResponse) GetAppKeys() []*AppKeysMetadataAppKeysAppKey { return v.AppKeys } +// AppMembersApp includes the requested fields of the GraphQL type App. +// The GraphQL type's documentation follows. +// +// A runtime caller identity owned by exactly one Organization or User. It owns +// long-lived App Keys and installs Agents through the AppAgent N:M join: one +// App may install many Agents, and one Agent may be installed in many Apps. +// The singular agentId / agent fields below are soft-deprecated convenience +// reads of the first install, not a direct foreign key. +type AppMembersApp struct { + Id string `json:"id"` + Urn string `json:"urn"` + Members []*AppMembersAppMembersAppMember `json:"members"` +} + +// GetId returns AppMembersApp.Id, and is useful for accessing the field via an interface. +func (v *AppMembersApp) GetId() string { return v.Id } + +// GetUrn returns AppMembersApp.Urn, and is useful for accessing the field via an interface. +func (v *AppMembersApp) GetUrn() string { return v.Urn } + +// GetMembers returns AppMembersApp.Members, and is useful for accessing the field via an interface. +func (v *AppMembersApp) GetMembers() []*AppMembersAppMembersAppMember { return v.Members } + +// AppMembersAppMembersAppMember includes the requested fields of the GraphQL type AppMember. +type AppMembersAppMembersAppMember struct { + Role string `json:"role"` + CreatedAt string `json:"createdAt"` + User *AppMembersAppMembersAppMemberUser `json:"user"` +} + +// GetRole returns AppMembersAppMembersAppMember.Role, and is useful for accessing the field via an interface. +func (v *AppMembersAppMembersAppMember) GetRole() string { return v.Role } + +// GetCreatedAt returns AppMembersAppMembersAppMember.CreatedAt, and is useful for accessing the field via an interface. +func (v *AppMembersAppMembersAppMember) GetCreatedAt() string { return v.CreatedAt } + +// GetUser returns AppMembersAppMembersAppMember.User, and is useful for accessing the field via an interface. +func (v *AppMembersAppMembersAppMember) GetUser() *AppMembersAppMembersAppMemberUser { return v.User } + +// AppMembersAppMembersAppMemberUser includes the requested fields of the GraphQL type User. +type AppMembersAppMembersAppMemberUser struct { + Id string `json:"id"` + Handle *string `json:"handle"` + // Canonical `hrn:user:` URN (spec cor:urn:010:01). Computed from the + // handle; null for a handle-less user (who has no URN). + Urn *string `json:"urn"` +} + +// GetId returns AppMembersAppMembersAppMemberUser.Id, and is useful for accessing the field via an interface. +func (v *AppMembersAppMembersAppMemberUser) GetId() string { return v.Id } + +// GetHandle returns AppMembersAppMembersAppMemberUser.Handle, and is useful for accessing the field via an interface. +func (v *AppMembersAppMembersAppMemberUser) GetHandle() *string { return v.Handle } + +// GetUrn returns AppMembersAppMembersAppMemberUser.Urn, and is useful for accessing the field via an interface. +func (v *AppMembersAppMembersAppMemberUser) GetUrn() *string { return v.Urn } + +// AppMembersResponse is returned by AppMembers on success. +type AppMembersResponse struct { + // Fetch an App (member of the App's org, or platform ADMIN — the myApps + // exposure bar; #473 relaxed this from org ADMIN for read parity with the + // list). + // + // 'ref' accepts the entity's ID or URN. + App *AppMembersApp `json:"app"` +} + +// GetApp returns AppMembersResponse.App, and is useful for accessing the field via an interface. +func (v *AppMembersResponse) GetApp() *AppMembersApp { return v.App } + // AppRunAppRun includes the requested fields of the GraphQL type AppRun. // The GraphQL type's documentation follows. // @@ -22056,6 +22126,62 @@ func (v *EndTeamSessionWithResumeClaimResponse) GetEndSession() *EndTeamSessionW return v.EndSession } +// EnsureAppMemberEnsureAppMember includes the requested fields of the GraphQL type AppMember. +type EnsureAppMemberEnsureAppMember struct { + Role string `json:"role"` + CreatedAt string `json:"createdAt"` + User *EnsureAppMemberEnsureAppMemberUser `json:"user"` +} + +// GetRole returns EnsureAppMemberEnsureAppMember.Role, and is useful for accessing the field via an interface. +func (v *EnsureAppMemberEnsureAppMember) GetRole() string { return v.Role } + +// GetCreatedAt returns EnsureAppMemberEnsureAppMember.CreatedAt, and is useful for accessing the field via an interface. +func (v *EnsureAppMemberEnsureAppMember) GetCreatedAt() string { return v.CreatedAt } + +// GetUser returns EnsureAppMemberEnsureAppMember.User, and is useful for accessing the field via an interface. +func (v *EnsureAppMemberEnsureAppMember) GetUser() *EnsureAppMemberEnsureAppMemberUser { return v.User } + +// EnsureAppMemberEnsureAppMemberUser includes the requested fields of the GraphQL type User. +type EnsureAppMemberEnsureAppMemberUser struct { + Id string `json:"id"` + Handle *string `json:"handle"` + // Canonical `hrn:user:` URN (spec cor:urn:010:01). Computed from the + // handle; null for a handle-less user (who has no URN). + Urn *string `json:"urn"` +} + +// GetId returns EnsureAppMemberEnsureAppMemberUser.Id, and is useful for accessing the field via an interface. +func (v *EnsureAppMemberEnsureAppMemberUser) GetId() string { return v.Id } + +// GetHandle returns EnsureAppMemberEnsureAppMemberUser.Handle, and is useful for accessing the field via an interface. +func (v *EnsureAppMemberEnsureAppMemberUser) GetHandle() *string { return v.Handle } + +// GetUrn returns EnsureAppMemberEnsureAppMemberUser.Urn, and is useful for accessing the field via an interface. +func (v *EnsureAppMemberEnsureAppMemberUser) GetUrn() *string { return v.Urn } + +// EnsureAppMemberResponse is returned by EnsureAppMember on success. +type EnsureAppMemberResponse struct { + // Idempotent UPSERT of an AppMember row. Per spec 008-agent-installation + // FR-004 / FR-016. The role MUST be a value present in the parent + // Agent's installationPolicy.memberRoles. Creating a new member is + // rejected if the App's current member count meets or exceeds the + // Agent's installationPolicy.maxMembers. Updating an existing member's + // role does NOT trigger the maxMembers check. The primary installed Agent + // (the oldest AppAgent) supplies this policy; no installed Agent refuses + // OrphanAppError. Only an App member manager may call this: the user-owner, + // an owner-role AppMember, or owning-org ADMIN (including platform admins + // for org-owned Apps). The manager gate runs before target-user lookup. + // + // Accepts the entity's ID or URN. + EnsureAppMember *EnsureAppMemberEnsureAppMember `json:"ensureAppMember"` +} + +// GetEnsureAppMember returns EnsureAppMemberResponse.EnsureAppMember, and is useful for accessing the field via an interface. +func (v *EnsureAppMemberResponse) GetEnsureAppMember() *EnsureAppMemberEnsureAppMember { + return v.EnsureAppMember +} + // ExtractParentNodeToMemoryExtractParentNodeToMemory includes the requested fields of the GraphQL type Memory. // The GraphQL type's documentation follows. // @@ -37609,6 +37735,20 @@ func (v *ReleaseWorkerResponse) GetReleaseWorker() *ReleaseWorkerReleaseWorkerRe return v.ReleaseWorker } +// RemoveAppMemberResponse is returned by RemoveAppMember on success. +type RemoveAppMemberResponse struct { + // Delete an AppMember row. Idempotent (no-op when the row doesn't exist). + // Personal-class memory at (appId, userId) is retained as an orphan per + // FR-015 — re-attaches automatically if the user later rejoins the same + // App. + // + // Accepts the entity's ID or URN. + RemoveAppMember *bool `json:"removeAppMember"` +} + +// GetRemoveAppMember returns RemoveAppMemberResponse.RemoveAppMember, and is useful for accessing the field via an interface. +func (v *RemoveAppMemberResponse) GetRemoveAppMember() *bool { return v.RemoveAppMember } + // RemoveMemoryMemberRemoveMemoryMemberRemoveMemoryMemberPayload includes the requested fields of the GraphQL type RemoveMemoryMemberPayload. type RemoveMemoryMemberRemoveMemoryMemberRemoveMemoryMemberPayload struct { MemoryId string `json:"memoryId"` @@ -61271,6 +61411,14 @@ type __AppKeysMetadataInput struct { // GetAppRef returns __AppKeysMetadataInput.AppRef, and is useful for accessing the field via an interface. func (v *__AppKeysMetadataInput) GetAppRef() string { return v.AppRef } +// __AppMembersInput is used internally by genqlient +type __AppMembersInput struct { + AppRef string `json:"appRef"` +} + +// GetAppRef returns __AppMembersInput.AppRef, and is useful for accessing the field via an interface. +func (v *__AppMembersInput) GetAppRef() string { return v.AppRef } + // __AppRunInput is used internally by genqlient type __AppRunInput struct { Ref string `json:"ref"` @@ -63021,6 +63169,22 @@ func (v *__EndTeamSessionWithResumeClaimInput) GetSummary() *string { return v.S // GetHandoff returns __EndTeamSessionWithResumeClaimInput.Handoff, and is useful for accessing the field via an interface. func (v *__EndTeamSessionWithResumeClaimInput) GetHandoff() *string { return v.Handoff } +// __EnsureAppMemberInput is used internally by genqlient +type __EnsureAppMemberInput struct { + AppRef string `json:"appRef"` + UserRef string `json:"userRef"` + Role string `json:"role"` +} + +// GetAppRef returns __EnsureAppMemberInput.AppRef, and is useful for accessing the field via an interface. +func (v *__EnsureAppMemberInput) GetAppRef() string { return v.AppRef } + +// GetUserRef returns __EnsureAppMemberInput.UserRef, and is useful for accessing the field via an interface. +func (v *__EnsureAppMemberInput) GetUserRef() string { return v.UserRef } + +// GetRole returns __EnsureAppMemberInput.Role, and is useful for accessing the field via an interface. +func (v *__EnsureAppMemberInput) GetRole() string { return v.Role } + // __ExtractParentNodeToMemoryInput is used internally by genqlient type __ExtractParentNodeToMemoryInput struct { ParentRef string `json:"parentRef"` @@ -64123,6 +64287,18 @@ func (v *__ReleaseWorkerInput) GetExpectedHolderUserId() *string { return v.Expe // GetExpectUnheld returns __ReleaseWorkerInput.ExpectUnheld, and is useful for accessing the field via an interface. func (v *__ReleaseWorkerInput) GetExpectUnheld() *bool { return v.ExpectUnheld } +// __RemoveAppMemberInput is used internally by genqlient +type __RemoveAppMemberInput struct { + AppRef string `json:"appRef"` + UserRef string `json:"userRef"` +} + +// GetAppRef returns __RemoveAppMemberInput.AppRef, and is useful for accessing the field via an interface. +func (v *__RemoveAppMemberInput) GetAppRef() string { return v.AppRef } + +// GetUserRef returns __RemoveAppMemberInput.UserRef, and is useful for accessing the field via an interface. +func (v *__RemoveAppMemberInput) GetUserRef() string { return v.UserRef } + // __RemoveMemoryMemberInput is used internally by genqlient type __RemoveMemoryMemberInput struct { MemoryRef string `json:"memoryRef"` @@ -68423,6 +68599,53 @@ func AppKeysMetadata( return data_, err_ } +// The query executed by AppMembers. +const AppMembers_Operation = ` +query AppMembers ($appRef: ID!) { + app(ref: $appRef) { + id + urn + members { + role + createdAt + user { + id + handle + urn + } + } + } +} +` + +// The visible members only. User.name / User.email are null when withheld, and +// are deliberately not selected. App.members is [] when the App's nested read is +// unavailable, so an empty list never proves the App has no members. +func AppMembers( + ctx_ context.Context, + client_ graphql.Client, + appRef string, +) (data_ *AppMembersResponse, err_ error) { + req_ := &graphql.Request{ + OpName: "AppMembers", + Query: AppMembers_Operation, + Variables: &__AppMembersInput{ + AppRef: appRef, + }, + } + + data_ = &AppMembersResponse{} + resp_ := &graphql.Response{Data: data_} + + err_ = client_.MakeRequest( + ctx_, + req_, + resp_, + ) + + return data_, err_ +} + // The query executed by AppRun. const AppRun_Operation = ` query AppRun ($ref: ID!) { @@ -74305,6 +74528,52 @@ func EndTeamSessionWithResumeClaim( return data_, err_ } +// The mutation executed by EnsureAppMember. +const EnsureAppMember_Operation = ` +mutation EnsureAppMember ($appRef: String!, $userRef: String!, $role: String!) { + ensureAppMember(appRef: $appRef, userRef: $userRef, role: $role) { + role + createdAt + user { + id + handle + urn + } + } +} +` + +// An idempotent upsert: the same role is a repeat, a different role changes the +// membership. The selection is the smallest that names the row. +func EnsureAppMember( + ctx_ context.Context, + client_ graphql.Client, + appRef string, + userRef string, + role string, +) (data_ *EnsureAppMemberResponse, err_ error) { + req_ := &graphql.Request{ + OpName: "EnsureAppMember", + Query: EnsureAppMember_Operation, + Variables: &__EnsureAppMemberInput{ + AppRef: appRef, + UserRef: userRef, + Role: role, + }, + } + + data_ = &EnsureAppMemberResponse{} + resp_ := &graphql.Response{Data: data_} + + err_ = client_.MakeRequest( + ctx_, + req_, + resp_, + ) + + return data_, err_ +} + // The mutation executed by ExtractParentNodeToMemory. const ExtractParentNodeToMemory_Operation = ` mutation ExtractParentNodeToMemory ($parentRef: ID!, $targetUrn: String!, $move: Boolean) { @@ -79680,6 +79949,43 @@ func ReleaseWorker( return data_, err_ } +// The mutation executed by RemoveAppMember. +const RemoveAppMember_Operation = ` +mutation RemoveAppMember ($appRef: String!, $userRef: String!) { + removeAppMember(appRef: $appRef, userRef: $userRef) +} +` + +// true whether or not a row existed (a deleteMany server-side), so the result +// says nothing about prior membership. A pointer so an absent answer (null, +// e.g. beside an error) stays distinguishable from an answered false. +func RemoveAppMember( + ctx_ context.Context, + client_ graphql.Client, + appRef string, + userRef string, +) (data_ *RemoveAppMemberResponse, err_ error) { + req_ := &graphql.Request{ + OpName: "RemoveAppMember", + Query: RemoveAppMember_Operation, + Variables: &__RemoveAppMemberInput{ + AppRef: appRef, + UserRef: userRef, + }, + } + + data_ = &RemoveAppMemberResponse{} + resp_ := &graphql.Response{Data: data_} + + err_ = client_.MakeRequest( + ctx_, + req_, + resp_, + ) + + return data_, err_ +} + // The mutation executed by RemoveMemoryMember. const RemoveMemoryMember_Operation = ` mutation RemoveMemoryMember ($memoryRef: String!, $userRef: String!) { diff --git a/internal/api/queries/app_members.graphql b/internal/api/queries/app_members.graphql new file mode 100644 index 00000000..4892577e --- /dev/null +++ b/internal/api/queries/app_members.graphql @@ -0,0 +1,46 @@ +# App human-member management (#956). userRef is an id / email / handle / +# hrn:user: URN passed through unchanged — the server resolves it, and the CLI +# never pre-resolves one (that would make a lookup a user-existence oracle). +# The role is a free string validated against the primary Agent's policy, so +# the CLI never lists or checks roles itself. + +# An idempotent upsert: the same role is a repeat, a different role changes the +# membership. The selection is the smallest that names the row. +mutation EnsureAppMember($appRef: String!, $userRef: String!, $role: String!) { + ensureAppMember(appRef: $appRef, userRef: $userRef, role: $role) { + role + createdAt + user { + id + handle + urn + } + } +} + +# true whether or not a row existed (a deleteMany server-side), so the result +# says nothing about prior membership. A pointer so an absent answer (null, +# e.g. beside an error) stays distinguishable from an answered false. +mutation RemoveAppMember($appRef: String!, $userRef: String!) { + # @genqlient(pointer: true) + removeAppMember(appRef: $appRef, userRef: $userRef) +} + +# The visible members only. User.name / User.email are null when withheld, and +# are deliberately not selected. App.members is [] when the App's nested read is +# unavailable, so an empty list never proves the App has no members. +query AppMembers($appRef: ID!) { + app(ref: $appRef) { + id + urn + members { + role + createdAt + user { + id + handle + urn + } + } + } +} diff --git a/internal/api/unbound-ops.txt b/internal/api/unbound-ops.txt index 7fe3c5e9..b8c16206 100644 --- a/internal/api/unbound-ops.txt +++ b/internal/api/unbound-ops.txt @@ -67,7 +67,7 @@ verifyDecision # server main schema refresh for cli#836; separate CLI parity ou verifyMemoryNodes # cli#802 exposes per-node verifyNode; memory-wide integrity audit needs a separate memory verify command workerWakeSettings # wake-surface v2; cli#828 tracks the paged attention surface, wake settings follow it -[Mutation] 109 unbound +[Mutation] 107 unbound acknowledgeWakes # wake-surface v2 (cor:agt:080:16); cli#828 lane acquireEditorLock addAgentToApp @@ -110,7 +110,6 @@ emailSaveDraft emailSendDraft emailUpdateDraft endAgentSession -ensureAppMember exchangeMoveMessage exchangeReply exportNodeToGoogleDoc @@ -135,7 +134,6 @@ registerContextResetRecovery # server#2162 context-reset host protocol; host ru releaseEditorLock removeAgentFromApp removeAgentImport -removeAppMember removeMemoryFromAgent repairAgentSystemMemory replaceSubtree # server#1407: deprecated field; authorized calls return REPLACE_SUBTREE_RETIRED without writes diff --git a/internal/cmd/agentic/agentic-usage.md b/internal/cmd/agentic/agentic-usage.md index f7d0e3a6..a55f1cae 100644 --- a/internal/cmd/agentic/agentic-usage.md +++ b/internal/cmd/agentic/agentic-usage.md @@ -1910,6 +1910,36 @@ Conventions: changes only the fields you pass (`--surface` replaces the set); `agent rm ` requires `--yes`. `` is an agent ID **or** a fully-qualified URN (`hrn:agent:acme.com:support-bot`) — no need to resolve an ID first. Memory-attach, AI-config wiring, and app-wiring land next. +- `app member add --user --role ` / `remove --user ` / `list` + (`ensureAppMember` / `removeAppMember` / `App.members`, `cor:dmo:050:04`) + manage an App's HUMAN members. The App is the root `--app` flag, else the + active App; there is no positional App, and an explicit blank `--app ""` + refuses (exit 2) rather than falling back to the active App. `--user` is an + id, email, handle or `hrn:user:` URN passed to the server UNCHANGED — never + resolved or trimmed client-side. `--role` is required on `add` (no default) + and is a server-owned string: help lists no roles, and an invalid role is + refused with the server's own list. `add` is an idempotent upsert (the same + role repeats; a different role changes the membership) and does no pre-read, + so EVERY `add` and `remove` is confirmed (the prompt names the App and whether + it came from `--app` or the active App; remove is reversible, so it says nothing + of "cannot be undone"): `--yes` skips the prompt, a + non-interactive call without it exits 2 with ZERO requests sent, and a + declined/EOF prompt exits 6. `remove` succeeds whether or not a row existed, + so `--json` reports `status: "absent"` (the state now), never "removed"; a + non-manager may remove only themselves, and any other target gets the + server's `Forbidden:` text as-is (no "user not found" — that would be an + existence oracle). A definite refusal keeps the server text and exits by code: + `InvalidRoleError` 2, `MaxMembersExceededError` and `OrphanAppError` 5, + not-found 4, auth 3, and the plain-text `Forbidden:` 1. ANY other failure of a write, or any result returned next to an error, + — transport error, `data` plus `errors`, a masked internal error, an empty + payload — exits 7 with "outcome unknown", prints no success DTO, and is never + retried: `ensureAppMember` re-reads the row after commit, so the write may + have landed; check with `app member list --app `. `list --json` is + `{appId, appUrn, members: [{user: {id, handle, urn}, role, createdAt}]}` + with `members: []` (never null) when empty. The server returns `[]` when the + member list is unreadable by you, so an empty list means "no members visible + to you", NOT "no members"; a null App is exit 4, a malformed row is an error. + User names and emails are withheld by the server and not selected. - `app agent add ` / `remove ` wrap `installAgentIntoApp` / `uninstallAgentFromApp` — the AppAgent join (`cor:dmo:050:03`), and the only way to attach an Agent to an App you diff --git a/internal/cmd/app/app.go b/internal/cmd/app/app.go index 0cd5d75e..86e5f87c 100644 --- a/internal/cmd/app/app.go +++ b/internal/cmd/app/app.go @@ -25,6 +25,7 @@ func NewCmdApp(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(newCmdUpdate(f)) cmd.AddCommand(newCmdUninstall(f)) cmd.AddCommand(newCmdKey(f)) + cmd.AddCommand(newCmdMember(f)) return cmd } diff --git a/internal/cmd/app/member.go b/internal/cmd/app/member.go new file mode 100644 index 00000000..5e95acac --- /dev/null +++ b/internal/cmd/app/member.go @@ -0,0 +1,354 @@ +package app + +import ( + "fmt" + "io" + "strconv" + "strings" + "unicode" + + "github.com/spf13/cobra" + + "github.com/hadron-memory/hadron-cli/internal/api" + "github.com/hadron-memory/hadron-cli/internal/api/gen" + "github.com/hadron-memory/hadron-cli/internal/cmdutil" + "github.com/hadron-memory/hadron-cli/internal/exitcode" + "github.com/hadron-memory/hadron-cli/internal/output" +) + +// appMemberUserDTO identifies a member without the fields the server withholds +// (name, email): only id, handle and URN are selected. +type appMemberUserDTO struct { + ID string `json:"id"` + Handle *string `json:"handle"` + URN *string `json:"urn"` +} + +// appMemberRowDTO is one visible member in `app member list`. +type appMemberRowDTO struct { + User appMemberUserDTO `json:"user"` + Role string `json:"role"` + CreatedAt string `json:"createdAt"` +} + +// appMemberListDTO is the stable --json shape of `app member list`. Members is +// the VISIBLE rows: an empty list never proves the App has no members. +type appMemberListDTO struct { + AppID string `json:"appId"` + AppURN string `json:"appUrn"` + Members []appMemberRowDTO `json:"members"` +} + +// appMemberAddDTO is the stable --json shape of a confirmed add/upsert. +type appMemberAddDTO struct { + App string `json:"app"` + UserRef string `json:"userRef"` + User appMemberUserDTO `json:"user"` + Role string `json:"role"` + CreatedAt string `json:"createdAt"` + Status string `json:"status"` +} + +// appMemberRemoveDTO is the stable --json shape of a remove. The server answers +// true whether or not a row existed, so Status is "absent" — the state now — +// never "removed". +type appMemberRemoveDTO struct { + App string `json:"app"` + UserRef string `json:"userRef"` + Status string `json:"status"` +} + +func newCmdMember(f *cmdutil.Factory) *cobra.Command { + cmd := &cobra.Command{ + Use: "member ", + Aliases: []string{"members"}, + Short: "Manage an App's human members", + Long: `Add, remove and list the human members of an App (cor:dmo:050:04). + +The App is the root --app flag, else the active App (` + "`hadron app use`" + `); +there is no positional App. --user is passed to the server unchanged — an id, +email, handle or hrn:user: URN — and is never resolved here. + +Roles are owned by the server (the App's primary Agent policy), so no role list +is shown or checked locally; an invalid role is refused with the server's own +list. Authority is the server's too: an App member manager, or the member +themselves for remove.`, + } + cmd.AddCommand(newCmdMemberList(f)) + cmd.AddCommand(newCmdMemberAdd(f)) + cmd.AddCommand(newCmdMemberRemove(f)) + return cmd +} + +// memberTargetApp resolves the ONE App a member command acts on. The shared +// resolver cannot be used as-is: Factory.App treats an explicit blank `--app ""` +// exactly like no flag and falls back to the active App, which would silently +// aim a membership write at an App the caller never named. An explicit blank +// refuses; an absent flag uses the active App; no App at all refuses. +// +// It also returns which branch answered ("--app" or "the active App") so the +// prompt and receipt can say so: a membership write aimed at an ambient App +// must not look like one aimed at a named one. +func memberTargetApp(cmd *cobra.Command, f *cmdutil.Factory) (app, source string, err error) { + explicit := false + if fl := cmd.Flag("app"); fl != nil && fl.Changed { + explicit = true + if strings.TrimSpace(f.AppFlag) == "" { + return "", "", exitcode.Newf(exitcode.Usage, "--app was given a blank value — name the App, or omit --app to use the active App") + } + } + app, err = f.App() + if err != nil { + return "", "", err + } + if app == "" { + return "", "", exitcode.Newf(exitcode.Usage, "an App is required: pass --app or select one with `hadron app use`") + } + if explicit { + return app, "from --app", nil + } + return app, "from the active App", nil +} + +// requireMemberUser refuses a blank --user. The value itself is passed on +// unchanged — no trimming, case folding or resolving. +func requireMemberUser(user string) error { + if strings.TrimSpace(user) == "" { + return exitcode.Newf(exitcode.Usage, "--user is required: an id, email, handle or hrn:user: URN") + } + return nil +} + +// memberWriteError classifies a failed membership write. A definite refusal +// keeps the server's wording and exit code; everything else is outcome UNKNOWN +// (see api.AppMemberWriteRefused) — non-zero, no success DTO, never retried. +// +// `partial` is true when the response carried a result next to the error: the +// resolver ran and returned data, so the write may have committed whatever the +// error says. The recovery command repeats the frozen target App. +func memberWriteError(err error, partial bool, appRef string) error { + if !partial && api.AppMemberWriteRefused(err) { + return api.MapError(err) + } + return exitcode.Newf(exitcode.Unavailable, + "outcome unknown: %s. The membership change may or may not have been applied and was not retried — "+ + "run %s to check (a list showing no row is not proof: rows you cannot see are withheld)", + api.MapError(err), memberListHint(appRef)) +} + +// memberListHint is the copy-pasteable check for the App a write was aimed at. +func memberListHint(appRef string) string { + return "`hadron app member list --app " + shellQuote(appRef) + "`" +} + +func shellQuote(s string) string { + if s != "" && strings.Trim(s, "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789:._/@-") == "" { + return s + } + return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" +} + +func memberUserDTO(id string, handle, urn *string) appMemberUserDTO { + return appMemberUserDTO{ID: id, Handle: handle, URN: urn} +} + +func newCmdMemberList(f *cmdutil.Factory) *cobra.Command { + return &cobra.Command{ + Use: "list", + Aliases: []string{"ls"}, + Short: "List the members of an App that you can see", + Long: `List the members of the App (--app, else the active App) that you can see. + +An empty list means "no members visible to you", never "no members": the server +returns [] when the App's member list is not readable by you.`, + Example: ` hadron app member list + hadron app member list --app hrn:app:acme.com:eng-team --json`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + appRef, source, err := memberTargetApp(cmd, f) + if err != nil { + return err + } + client, err := f.GraphQLClient() + if err != nil { + return err + } + resp, err := gen.AppMembers(cmd.Context(), client, appRef) + if err != nil { + return api.MapError(err) + } + if resp == nil || resp.App == nil { + return exitcode.Newf(exitcode.NotFound, "app %q not found, or not visible to you", appRef) + } + dto := appMemberListDTO{AppID: resp.App.Id, AppURN: resp.App.Urn, Members: []appMemberRowDTO{}} + for _, m := range resp.App.Members { + if m == nil || m.User == nil { + return exitcode.Newf(exitcode.Error, "server returned a malformed member row for %q", appRef) + } + dto.Members = append(dto.Members, appMemberRowDTO{ + User: memberUserDTO(m.User.Id, m.User.Handle, m.User.Urn), + Role: m.Role, + CreatedAt: m.CreatedAt, + }) + } + return output.Write(f.IOStreams, f.JSON, dto, func(w io.Writer) error { + if _, err := fmt.Fprintf(w, "app: %s (%s)\n", memberText(dto.AppURN), source); err != nil { + return err + } + if len(dto.Members) == 0 { + _, err := fmt.Fprintln(w, "no members visible to you") + return err + } + t := output.NewTable(w, "USER ID", "HANDLE", "ROLE", "ADDED") + for _, m := range dto.Members { + t.Row(m.User.ID, memberCell(m.User.Handle), memberText(m.Role), m.CreatedAt) + } + return t.Flush() + }) + }, + } +} + +func newCmdMemberAdd(f *cmdutil.Factory) *cobra.Command { + var user, role string + var yes bool + cmd := &cobra.Command{ + Use: "add --user --role ", + Short: "Add a user to an App, or change their role", + Long: `Add a user to the App (--app, else the active App) with the given role. + +This is an idempotent upsert: the same role is a repeat, a different role +changes the membership. Because the command never reads the list first, EVERY +add is confirmed — pass --yes to skip the prompt; a non-interactive caller +without --yes is refused before anything is sent. + +If the response is lost or ambiguous the outcome is reported as unknown and the +write is not retried; check with ` + "`hadron app member list`" + `.`, + Example: ` hadron app member add --user ada@acme.com --role member --yes + hadron app member add --app hrn:app:acme.com:eng-team --user hrn:user:ada --role owner`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + if err := requireMemberUser(user); err != nil { + return err + } + if strings.TrimSpace(role) == "" { + return exitcode.Newf(exitcode.Usage, "--role is required: roles are defined by the server and no default is assumed") + } + appRef, source, err := memberTargetApp(cmd, f) + if err != nil { + return err + } + prompt := fmt.Sprintf("Add user %s to app %s (%s) with role %s?", strconv.Quote(user), appRef, source, strconv.Quote(role)) + if err := cmdutil.Confirm(f.IOStreams, yes, prompt); err != nil { + return err + } + client, err := f.GraphQLClient() + if err != nil { + return err + } + resp, err := gen.EnsureAppMember(cmd.Context(), client, appRef, user, role) + if err != nil { + return memberWriteError(err, resp != nil && resp.EnsureAppMember != nil, appRef) + } + if resp == nil || resp.EnsureAppMember == nil || resp.EnsureAppMember.User == nil { + return exitcode.Newf(exitcode.Unavailable, + "outcome unknown: the server returned no member row. Run %s to check (a list showing no row is not proof: rows you cannot see are withheld)", memberListHint(appRef)) + } + m := resp.EnsureAppMember + dto := appMemberAddDTO{ + App: appRef, + UserRef: user, + User: memberUserDTO(m.User.Id, m.User.Handle, m.User.Urn), + Role: m.Role, + CreatedAt: m.CreatedAt, + Status: "member", + } + return output.Write(f.IOStreams, f.JSON, dto, func(w io.Writer) error { + _, err := fmt.Fprintf(w, "✓ %s is a member of %s (%s) with role %s\n", memberText(user), appRef, source, memberText(dto.Role)) + return err + }) + }, + } + cmd.Flags().StringVar(&user, "user", "", "user to add: an id, email, handle or hrn:user: URN, passed to the server unchanged (required)") + cmd.Flags().StringVar(&role, "role", "", "the member's role, as the server defines roles (required)") + cmd.Flags().BoolVarP(&yes, "yes", "y", false, "skip the confirmation prompt") + return cmd +} + +func newCmdMemberRemove(f *cmdutil.Factory) *cobra.Command { + var user string + var yes bool + cmd := &cobra.Command{ + Use: "remove --user ", + Aliases: []string{"rm"}, + Short: "Remove a user from an App", + Long: `Remove a user from the App (--app, else the active App). Reversible: add the +user again to restore the membership. + +The server answers the same whether or not the user was a member, so success +means "not a member now", not "was removed". An App member manager may remove +anyone; anyone else may remove only themselves, and every other target gets the +same refusal — which is printed as-is, so it cannot be used to test whether a +user exists. + +Confirmed first: pass --yes to skip the prompt; a non-interactive caller +without --yes is refused before anything is sent.`, + Example: ` hadron app member remove --user ada@acme.com --yes`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + if err := requireMemberUser(user); err != nil { + return err + } + appRef, source, err := memberTargetApp(cmd, f) + if err != nil { + return err + } + // Confirm, not ConfirmDeletion: removal is reversible (add the user + // again; their personal memory is retained), so "cannot be undone" + // would be false. + prompt := fmt.Sprintf("Remove user %s from app %s (%s)?", strconv.Quote(user), appRef, source) + if err := cmdutil.Confirm(f.IOStreams, yes, prompt); err != nil { + return err + } + client, err := f.GraphQLClient() + if err != nil { + return err + } + resp, err := gen.RemoveAppMember(cmd.Context(), client, appRef, user) + if err != nil { + return memberWriteError(err, resp != nil && resp.RemoveAppMember != nil, appRef) + } + if resp == nil || resp.RemoveAppMember == nil || !*resp.RemoveAppMember { + return exitcode.Newf(exitcode.Unavailable, + "outcome unknown: the server did not confirm the removal. Run %s to check (a list showing no row is not proof: rows you cannot see are withheld)", memberListHint(appRef)) + } + dto := appMemberRemoveDTO{App: appRef, UserRef: user, Status: "absent"} + return output.Write(f.IOStreams, f.JSON, dto, func(w io.Writer) error { + _, err := fmt.Fprintf(w, "✓ %s is not a member of %s (%s) now\n", memberText(user), appRef, source) + return err + }) + }, + } + cmd.Flags().StringVar(&user, "user", "", "user to remove: an id, email, handle or hrn:user: URN, passed to the server unchanged (required)") + cmd.Flags().BoolVarP(&yes, "yes", "y", false, "skip the confirmation prompt") + return cmd +} + +// memberText quotes a string carrying control characters (newline, ESC…) so a +// server- or user-supplied value cannot split a row or reach the terminal as an +// escape sequence. Text only: --json keeps the source value. +func memberText(s string) string { + for _, r := range s { + if unicode.IsControl(r) { + return strconv.Quote(s) + } + } + return s +} + +func memberCell(s *string) string { + if s == nil || *s == "" { + return "—" + } + return memberText(*s) +} diff --git a/internal/cmd/app_member_cmd_test.go b/internal/cmd/app_member_cmd_test.go new file mode 100644 index 00000000..f2ccfd7f --- /dev/null +++ b/internal/cmd/app_member_cmd_test.go @@ -0,0 +1,576 @@ +package cmd + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/hadron-memory/hadron-cli/internal/config" + "github.com/hadron-memory/hadron-cli/internal/exitcode" +) + +// cli#956: `app member add|remove|list`. Every write test asserts how many +// mutation calls reached the server — the guards that matter (confirm before +// write, no retry, no pre-read) are all "zero or exactly one call". + +const ( + memberApp = "hrn:app:acme.com:eng-team" + memberActiveApp = "hrn:app:acme.com:active" +) + +const ensureMemberResp = `{"data":{"ensureAppMember":{"role":"member","createdAt":"2026-10-25T00:00:00Z", + "user":{"id":"usr1","handle":"ada","urn":"hrn:user:ada"}}}}` + +const removeMemberResp = `{"data":{"removeAppMember":true}}` + +const appMembersResp = `{"data":{"app":{"id":"capp100000000000000000000","urn":"hrn:app:acme.com:eng-team","members":[ + {"role":"owner","createdAt":"2026-10-01T00:00:00Z","user":{"id":"usr1","handle":"ada","urn":"hrn:user:ada"}}, + {"role":"member","createdAt":"2026-10-02T00:00:00Z","user":{"id":"usr2","handle":null,"urn":null}}]}}}` + +func memberGQLError(msg, code string) string { + b, _ := json.Marshal(map[string]any{"errors": []any{map[string]any{ + "message": msg, "extensions": map[string]any{"code": code}}}}) + return string(b) +} + +// countingGraphQL answers by operation name and counts calls per operation. +func countingGraphQL(t *testing.T, responses map[string]string) (*httptest.Server, func(op string) int, func(op string) json.RawMessage) { + t.Helper() + var mu sync.Mutex + calls := map[string]int{} + vars := map[string]json.RawMessage{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && r.URL.Path == "/capabilities" { + _, _ = w.Write([]byte(`{"capabilities":[]}`)) + return + } + var body struct { + OperationName string `json:"operationName"` + Variables json.RawMessage `json:"variables"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + mu.Lock() + calls[body.OperationName]++ + vars[body.OperationName] = body.Variables + mu.Unlock() + resp, ok := responses[body.OperationName] + if !ok { + t.Errorf("unexpected operation %q", body.OperationName) + resp = memberGQLError("unexpected operation", "INTERNAL_SERVER_ERROR") + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(resp)) + })) + t.Cleanup(srv.Close) + return srv, + func(op string) int { mu.Lock(); defer mu.Unlock(); return calls[op] }, + func(op string) json.RawMessage { mu.Lock(); defer mu.Unlock(); return vars[op] } +} + +func setActiveApp(t *testing.T, ref string) { + t.Helper() + path, err := config.File() + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("app = \""+ref+"\"\n"), 0o600); err != nil { + t.Fatal(err) + } +} + +func memberVar(t *testing.T, raw json.RawMessage, key string) any { + t.Helper() + var v map[string]any + if err := json.Unmarshal(raw, &v); err != nil { + t.Fatalf("vars: %v (%s)", err, raw) + } + return v[key] +} + +// Test 1: every userRef form reaches the server byte-identical. +func TestAppMemberUserRefPassesThroughUnchanged(t *testing.T) { + for _, ref := range []string{"usr1", "Ada@Acme.COM", "Ada", "hrn:user:ada", " ada "} { + srv, _, vars := countingGraphQL(t, map[string]string{"EnsureAppMember": ensureMemberResp, "RemoveAppMember": removeMemberResp}) + for _, verb := range []string{"add", "remove"} { + f, _ := testFactory(t) + args := []string{"app", "member", verb, "--user", ref, "--app", memberApp, "--yes", "--json", "--server", srv.URL} + if verb == "add" { + args = append(args, "--role", "member") + } + root := NewRootCmd(f) + root.SetArgs(args) + if err := root.Execute(); err != nil { + t.Fatalf("%s %q: %v", verb, ref, err) + } + op := map[string]string{"add": "EnsureAppMember", "remove": "RemoveAppMember"}[verb] + if got := memberVar(t, vars(op), "userRef"); got != ref { + t.Errorf("%s: userRef %q reached the server as %q", verb, ref, got) + } + if got := memberVar(t, vars(op), "appRef"); got != memberApp { + t.Errorf("%s: appRef = %v", verb, got) + } + } + } +} + +// Test 2: explicit --app, the active App, and neither. +func TestAppMemberAppResolution(t *testing.T) { + cases := []struct { + name string + active string + flag []string + want string // "" = Usage refusal with zero calls + }{ + {"explicit", "", []string{"--app", memberApp}, memberApp}, + {"explicit beats active", memberActiveApp, []string{"--app", memberApp}, memberApp}, + {"active default", memberActiveApp, nil, memberActiveApp}, + {"neither", "", nil, ""}, + } + for _, verb := range []string{"add", "remove", "list"} { + for _, c := range cases { + t.Run(verb+"/"+c.name, func(t *testing.T) { + srv, calls, vars := countingGraphQL(t, map[string]string{ + "EnsureAppMember": ensureMemberResp, "RemoveAppMember": removeMemberResp, "AppMembers": appMembersResp}) + f, _ := testFactory(t) + if c.active != "" { + setActiveApp(t, c.active) + } + args := append([]string{"app", "member", verb}, c.flag...) + if verb != "list" { + args = append(args, "--user", "ada", "--yes") + } + if verb == "add" { + args = append(args, "--role", "member") + } + root := NewRootCmd(f) + root.SetArgs(append(args, "--json", "--server", srv.URL)) + err := root.Execute() + op := map[string]string{"add": "EnsureAppMember", "remove": "RemoveAppMember", "list": "AppMembers"}[verb] + if c.want == "" { + if exitCodeFor(err) != exitcode.Usage || calls(op) != 0 { + t.Fatalf("no App must be a Usage refusal with 0 calls: err=%v calls=%d", err, calls(op)) + } + return + } + if err != nil { + t.Fatal(err) + } + if got := memberVar(t, vars(op), "appRef"); got != c.want { + t.Errorf("appRef = %v, want %s", got, c.want) + } + }) + } + } +} + +// Test 3: an explicit blank --app refuses even when an active App is set. The +// shared resolver would silently use the active App. +func TestAppMemberExplicitBlankAppRefuses(t *testing.T) { + for _, blank := range []string{"", " "} { + for _, verb := range []string{"add", "remove", "list"} { + srv, calls, _ := countingGraphQL(t, map[string]string{ + "EnsureAppMember": ensureMemberResp, "RemoveAppMember": removeMemberResp, "AppMembers": appMembersResp}) + f, _ := testFactory(t) + setActiveApp(t, memberActiveApp) + args := []string{"app", "member", verb, "--app", blank, "--yes"} + if verb != "list" { + args = append(args, "--user", "ada") + } + if verb == "add" { + args = append(args, "--role", "member") + } + if verb == "list" { + args = args[:len(args)-1] // list takes no --yes + } + root := NewRootCmd(f) + root.SetArgs(append(args, "--server", srv.URL)) + err := root.Execute() + total := calls("EnsureAppMember") + calls("RemoveAppMember") + calls("AppMembers") + if exitCodeFor(err) != exitcode.Usage || total != 0 { + t.Errorf("%s --app %q: want Usage + 0 calls, got err=%v calls=%d", verb, blank, err, total) + } + } + } +} + +// Test 4: add without --role is a Usage error and invents no default. +func TestAppMemberAddRequiresRole(t *testing.T) { + srv, calls, _ := countingGraphQL(t, map[string]string{"EnsureAppMember": ensureMemberResp}) + f, _ := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "add", "--user", "ada", "--app", memberApp, "--yes", "--server", srv.URL}) + err := root.Execute() + if exitCodeFor(err) != exitcode.Usage || calls("EnsureAppMember") != 0 { + t.Fatalf("want Usage + 0 calls, got err=%v calls=%d", err, calls("EnsureAppMember")) + } + f2, _ := testFactory(t) + root = NewRootCmd(f2) + root.SetArgs([]string{"app", "member", "add", "--user", "ada", "--role", " ", "--app", memberApp, "--yes", "--server", srv.URL}) + if err := root.Execute(); exitCodeFor(err) != exitcode.Usage || calls("EnsureAppMember") != 0 { + t.Fatalf("blank role: want Usage + 0 calls, got err=%v", err) + } +} + +// Test 5: confirmation. Nothing is written before it. +func TestAppMemberConfirmation(t *testing.T) { + for _, verb := range []string{"add", "remove"} { + op := map[string]string{"add": "EnsureAppMember", "remove": "RemoveAppMember"}[verb] + base := []string{"app", "member", verb, "--user", "Ada@acme.com", "--app", memberApp} + if verb == "add" { + base = append(base, "--role", "owner") + } + run := func(t *testing.T, tty bool, answers string, extra ...string) (int, string, error) { + srv, calls, _ := countingGraphQL(t, map[string]string{"EnsureAppMember": ensureMemberResp, "RemoveAppMember": removeMemberResp}) + var errOut string + var err error + args := append(append([]string{}, base...), extra...) + args = append(args, "--server", srv.URL) + if tty { + f, _, eo := testFactoryTTY(t, answers) + root := NewRootCmd(f) + root.SetArgs(args) + err = root.Execute() + errOut = eo.String() + } else { + f, _ := testFactory(t) + root := NewRootCmd(f) + root.SetArgs(args) + err = root.Execute() + } + return calls(op), errOut, err + } + t.Run(verb+"/non-TTY without --yes", func(t *testing.T) { + n, _, err := run(t, false, "") + if exitCodeFor(err) != exitcode.Usage || n != 0 { + t.Fatalf("want Usage + 0 calls, got err=%v calls=%d", err, n) + } + }) + t.Run(verb+"/TTY no", func(t *testing.T) { + n, _, err := run(t, true, "n\n") + if exitCodeFor(err) != exitcode.Cancelled || n != 0 { + t.Fatalf("want Cancelled + 0 calls, got err=%v calls=%d", err, n) + } + }) + t.Run(verb+"/TTY EOF", func(t *testing.T) { + n, _, err := run(t, true, "") + if exitCodeFor(err) != exitcode.Cancelled || n != 0 { + t.Fatalf("want Cancelled + 0 calls, got err=%v calls=%d", err, n) + } + }) + t.Run(verb+"/TTY yes names app user role", func(t *testing.T) { + n, prompt, err := run(t, true, "y\n") + if err != nil || n != 1 { + t.Fatalf("want success + 1 call, got err=%v calls=%d", err, n) + } + for _, want := range []string{memberApp, `"Ada@acme.com"`} { + if !strings.Contains(prompt, want) { + t.Errorf("prompt must name %s: %q", want, prompt) + } + } + if strings.Contains(prompt, "cannot be undone") { + t.Errorf("a reversible membership change must not claim it cannot be undone: %q", prompt) + } + if !strings.Contains(prompt, "from --app") { + t.Errorf("prompt must name where the App came from: %q", prompt) + } + if verb == "add" && !strings.Contains(prompt, `"owner"`) { + t.Errorf("add prompt must name the role: %q", prompt) + } + }) + t.Run(verb+"/--yes", func(t *testing.T) { + n, prompt, err := run(t, true, "", "--yes") + if err != nil || n != 1 || prompt != "" { + t.Fatalf("want 1 call and no prompt, got err=%v calls=%d prompt=%q", err, n, prompt) + } + }) + } +} + +// Test 6: same-role repeat and changed-role add each make exactly one call, and +// nothing reads the list first. +func TestAppMemberAddNeverPreReads(t *testing.T) { + srv, calls, _ := countingGraphQL(t, map[string]string{"EnsureAppMember": ensureMemberResp}) + for _, role := range []string{"member", "member", "owner"} { + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "add", "--user", "ada", "--role", role, "--app", memberApp, "--yes", "--json", "--server", srv.URL}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + var dto struct{ Status, Role, UserRef string } + if err := json.Unmarshal([]byte(out.String()), &dto); err != nil || dto.Status != "member" { + t.Fatalf("dto: %v %s", err, out.String()) + } + } + if calls("EnsureAppMember") != 3 || calls("AppMembers") != 0 { + t.Errorf("want 3 upserts and no list read, got %d / %d", calls("EnsureAppMember"), calls("AppMembers")) + } +} + +// Test 7: remove is "absent now", never "removed", and repeats succeed. +func TestAppMemberRemoveReportsAbsentNotRemoved(t *testing.T) { + srv, calls, _ := countingGraphQL(t, map[string]string{"RemoveAppMember": removeMemberResp}) + for i := 0; i < 2; i++ { + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "remove", "--user", "ada", "--app", memberApp, "--yes", "--json", "--server", srv.URL}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + var dto struct{ Status string } + if err := json.Unmarshal([]byte(out.String()), &dto); err != nil || dto.Status != "absent" { + t.Fatalf("status must be absent: %v %s", err, out.String()) + } + f, out = testFactory(t) + root = NewRootCmd(f) + root.SetArgs([]string{"app", "member", "remove", "--user", "ada", "--app", memberApp, "--yes", "--server", srv.URL}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + if s := out.String(); strings.Contains(s, "removed") || !strings.Contains(s, "not a member") { + t.Errorf("text must say 'not a member', never 'removed': %q", s) + } + } + if calls("RemoveAppMember") != 4 { + t.Errorf("calls = %d", calls("RemoveAppMember")) + } +} + +// Test 8: typed refusals keep the server's text and are definite. +func TestAppMemberDefiniteRefusalsKeepServerText(t *testing.T) { + cases := []struct{ msg, code string }{ + {`role "boss" is not valid; allowed: owner, member`, "InvalidRoleError"}, + {"App already has the maximum number of members", "MaxMembersExceededError"}, + {"this would leave the App with no owner", "OrphanAppError"}, + {"Forbidden: only an App member manager may do this", "INTERNAL_SERVER_ERROR"}, + } + for _, c := range cases { + srv, calls, _ := countingGraphQL(t, map[string]string{"EnsureAppMember": memberGQLError(c.msg, c.code)}) + f, _ := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "add", "--user", "ada", "--role", "boss", "--app", memberApp, "--yes", "--server", srv.URL}) + err := root.Execute() + if err == nil || exitCodeFor(err) == 0 { + t.Fatalf("%s: want a non-zero refusal", c.code) + } + if !strings.Contains(err.Error(), c.msg) { + t.Errorf("%s: server text must be kept verbatim: %v", c.code, err) + } + if strings.Contains(strings.ToLower(err.Error()), "unknown") || calls("EnsureAppMember") != 1 { + t.Errorf("%s: a definite refusal must not read as outcome unknown / retry: %v calls=%d", c.code, err, calls("EnsureAppMember")) + } + } +} + +// Test 9: outcome unknown — non-zero, no success DTO, exactly one call, and it +// points at `app member list`. +func TestAppMemberOutcomeUnknown(t *testing.T) { + partial := `{"data":{"ensureAppMember":null},"errors":[{"message":"boom","extensions":{"code":"INTERNAL_SERVER_ERROR"}}]}` + nested := `{"data":{"ensureAppMember":{"role":"member","createdAt":"2026-10-25T00:00:00Z","user":{"id":"u","handle":null,"urn":"hrn:user:ada"}}},` + + `"errors":[{"message":"Forbidden: nested field not readable","path":["ensureAppMember","user","handle"],"extensions":{"code":"FORBIDDEN"}}]}` + cases := map[string]string{ + "partial data with nested FORBIDDEN": nested, + "nested-path error alone": `{"data":{"ensureAppMember":null},"errors":[{"message":"Forbidden: x","path":["ensureAppMember","user"],"extensions":{"code":"FORBIDDEN"}}]}`, + "post-write internal error": memberGQLError("Internal server error", "INTERNAL_SERVER_ERROR"), + "data and errors": partial, + "no code": `{"errors":[{"message":"mystery"}]}`, + "null payload": `{"data":{"ensureAppMember":null}}`, + "null user in row": `{"data":{"ensureAppMember":{"role":"member","createdAt":"2026-10-25T00:00:00Z","user":null}}}`, + "forbidden mid-text": memberGQLError("Internal: wrapped Forbidden: detail", "INTERNAL_SERVER_ERROR"), + "mixed with a refusal": `{"errors":[{"message":"x","extensions":{"code":"InvalidRoleError"}},{"message":"y","extensions":{"code":"INTERNAL_SERVER_ERROR"}}]}`, + } + for name, resp := range cases { + t.Run(name, func(t *testing.T) { + srv, calls, _ := countingGraphQL(t, map[string]string{"EnsureAppMember": resp}) + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "add", "--user", "ada", "--role", "member", "--app", memberApp, "--yes", "--json", "--server", srv.URL}) + err := root.Execute() + if err == nil { + t.Fatal("must exit non-zero") + } + if out.String() != "" { + t.Errorf("no success DTO on stdout: %q", out.String()) + } + if !strings.Contains(err.Error(), "outcome unknown") || !strings.Contains(err.Error(), "app member list --app "+memberApp) { + t.Errorf("must say outcome unknown and point at list: %v", err) + } + if calls("EnsureAppMember") != 1 { + t.Errorf("must not retry: %d calls", calls("EnsureAppMember")) + } + }) + } + t.Run("partial remove result", func(t *testing.T) { + srv, calls, _ := countingGraphQL(t, map[string]string{"RemoveAppMember": `{"data":{"removeAppMember":true},"errors":[{"message":"Forbidden: x","extensions":{"code":"FORBIDDEN"}}]}`}) + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "remove", "--user", "ada", "--app", memberApp, "--yes", "--server", srv.URL}) + err := root.Execute() + if err == nil || out.String() != "" || !strings.Contains(err.Error(), "outcome unknown") || calls("RemoveAppMember") != 1 { + t.Fatalf("want outcome unknown: err=%v out=%q", err, out.String()) + } + }) + t.Run("remove false beside a refusal", func(t *testing.T) { + srv, calls, _ := countingGraphQL(t, map[string]string{"RemoveAppMember": `{"data":{"removeAppMember":false},"errors":[{"message":"Forbidden: x","extensions":{"code":"FORBIDDEN"}}]}`}) + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "remove", "--user", "ada", "--app", memberApp, "--yes", "--server", srv.URL}) + err := root.Execute() + if err == nil || out.String() != "" || !strings.Contains(err.Error(), "outcome unknown") || calls("RemoveAppMember") != 1 { + t.Fatalf("a false result beside an error is partial data, not a definite refusal: err=%v out=%q", err, out.String()) + } + }) + t.Run("remove answered false", func(t *testing.T) { + srv, calls, _ := countingGraphQL(t, map[string]string{"RemoveAppMember": `{"data":{"removeAppMember":false}}`}) + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "remove", "--user", "ada", "--app", memberApp, "--yes", "--json", "--server", srv.URL}) + err := root.Execute() + if err == nil || out.String() != "" || !strings.Contains(err.Error(), "outcome unknown") || calls("RemoveAppMember") != 1 { + t.Fatalf("a false answer must not report absent: err=%v out=%q", err, out.String()) + } + }) + t.Run("transport failure", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/capabilities" { + _, _ = w.Write([]byte(`{"capabilities":[]}`)) + return + } + hj, _ := w.(http.Hijacker) + conn, _, _ := hj.Hijack() + _ = conn.Close() + })) + defer srv.Close() + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "remove", "--user", "ada", "--app", memberApp, "--yes", "--server", srv.URL}) + err := root.Execute() + if err == nil || out.String() != "" || !strings.Contains(err.Error(), "outcome unknown") { + t.Fatalf("want outcome unknown, no stdout: err=%v out=%q", err, out.String()) + } + }) +} + +// Test 10: a foreign / unknown remove target gets the server's Forbidden text +// as-is, never "not found" (that would reopen the user-existence oracle). +func TestAppMemberRemoveForbiddenIsNotTranslated(t *testing.T) { + msg := "Forbidden: you must manage this App or be the user themselves" + srv, _, _ := countingGraphQL(t, map[string]string{"RemoveAppMember": memberGQLError(msg, "INTERNAL_SERVER_ERROR")}) + f, _ := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "remove", "--user", "nobody@example.com", "--app", memberApp, "--yes", "--server", srv.URL}) + err := root.Execute() + if err == nil || !strings.Contains(err.Error(), msg) { + t.Fatalf("want the Forbidden text verbatim: %v", err) + } + if exitCodeFor(err) == exitcode.NotFound || strings.Contains(strings.ToLower(err.Error()), "not found") { + t.Errorf("must not be translated to not-found: %v", err) + } +} + +// Test 11: list. +func TestAppMemberList(t *testing.T) { + t.Run("rows", func(t *testing.T) { + srv, _, vars := countingGraphQL(t, map[string]string{"AppMembers": appMembersResp}) + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "list", "--app", memberApp, "--json", "--server", srv.URL}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + var dto struct { + AppURN string `json:"appUrn"` + Members []struct { + Role string `json:"role"` + User struct { + ID string `json:"id"` + Handle *string `json:"handle"` + } `json:"user"` + } `json:"members"` + } + if err := json.Unmarshal([]byte(out.String()), &dto); err != nil || len(dto.Members) != 2 || dto.Members[0].Role != "owner" || + dto.Members[1].User.Handle != nil || dto.AppURN != memberApp { + t.Fatalf("dto: %v %s", err, out.String()) + } + _ = vars + }) + t.Run("empty", func(t *testing.T) { + srv, _, _ := countingGraphQL(t, map[string]string{"AppMembers": `{"data":{"app":{"id":"a","urn":"hrn:app:acme.com:eng-team","members":[]}}}`}) + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "list", "--app", memberApp, "--server", srv.URL}) + if err := root.Execute(); err != nil || !strings.HasSuffix(strings.TrimSpace(out.String()), "no members visible to you") || + !strings.Contains(out.String(), "(from --app)") { + t.Fatalf("err=%v out=%q", err, out.String()) + } + f, out = testFactory(t) + root = NewRootCmd(f) + root.SetArgs([]string{"app", "member", "list", "--app", memberApp, "--json", "--server", srv.URL}) + if err := root.Execute(); err != nil || !strings.Contains(out.String(), `"members": []`) && !strings.Contains(out.String(), `"members":[]`) { + t.Fatalf("json must carry [] not null: %v %s", err, out.String()) + } + }) + t.Run("null app is not found", func(t *testing.T) { + srv, _, _ := countingGraphQL(t, map[string]string{"AppMembers": `{"data":{"app":null}}`}) + f, out := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "list", "--app", memberApp, "--json", "--server", srv.URL}) + err := root.Execute() + if exitCodeFor(err) != exitcode.NotFound || strings.Contains(out.String(), "members") { + t.Fatalf("want NotFound and no list: err=%v out=%q", err, out.String()) + } + }) + t.Run("malformed row is an error", func(t *testing.T) { + srv, _, _ := countingGraphQL(t, map[string]string{"AppMembers": `{"data":{"app":{"id":"a","urn":"u","members":[null]}}}`}) + f, _ := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "list", "--app", memberApp, "--server", srv.URL}) + if err := root.Execute(); err == nil { + t.Fatal("a malformed row must not become a successful list") + } + }) + t.Run("errors are errors", func(t *testing.T) { + srv, _, _ := countingGraphQL(t, map[string]string{"AppMembers": memberGQLError("nope", "FORBIDDEN")}) + f, _ := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "list", "--app", memberApp, "--server", srv.URL}) + if err := root.Execute(); exitCodeFor(err) != exitcode.Forbidden { + t.Fatalf("want Forbidden, got %v", err) + } + }) +} + +// The selection must never ask for the fields the server withholds. +func TestAppMemberQueriesSelectNoWithheldFields(t *testing.T) { + var query string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/capabilities" { + _, _ = w.Write([]byte(`{"capabilities":[]}`)) + return + } + var body struct{ Query string } + _ = json.NewDecoder(r.Body).Decode(&body) + query = body.Query + _, _ = w.Write([]byte(appMembersResp)) + })) + defer srv.Close() + f, _ := testFactory(t) + root := NewRootCmd(f) + root.SetArgs([]string{"app", "member", "list", "--app", memberApp, "--server", srv.URL}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + for _, field := range []string{"email", "name"} { + if strings.Contains(query, field) { + t.Errorf("query selects %q: %s", field, query) + } + } +}