Support reclaiming mannequins to customer-owned bot accounts

[dpmex4527]

This PR adds bot-target support to gh elm mannequin claim, so an organization admin or enterprise admin can reattribute migrated content from a bot mannequin to a customer-owned GitHub App / bot account (previously only human users were supported). Attribution to GitHub owned/first party apps is rejected on the backend.

Context

Customers whose automation moved from machine-user "bot accounts" to GitHub Apps couldn't reclaim their apps' bot-authored content. The reclaim path resolves the target via the GraphQL user(login:) query, which hides bots (it only resolves accounts where user? is true), so an app login like example-ci[bot] could not be resolved or reclaimed.

Notable Changes

• The CLI detects the [bot] suffix on the target and routes to a new ReattributeMannequinToBot mutation.
• Since bots can't be resolved through graphql, the CLI now uses rest API to fetch the bot gql ID
• The new mutation is under feature flag so we pass the mannequin_claiming_bot feature flag in the GraphQL-Features header
• Since bots can't receive emails, there is no manual confirmation flow. Only org admins and enterprise admins can call the new mutation and this mutation immediately reattributes the mannequin to bot after passing ownership checks
• Similar to the auto attribution process for users, the CLI displays prompts a confirmation step for both individual reclaim and CSV reclaim of bots warning users that mannequin reclamation is a one way step and that they need to confirm their intent to reclaim. It also has the --no-prompt flag to skip the prompt step
• If a user attempts to reclaim a mannequin bot that doesn't have the [bot] suffix, a warning is displayed. We allow this because non GitHub sources don't use GitHub bot conventions and we still want customers to be able to reclaim ADO/BBS bot accounts

Testing/screenshots

Reclaiming against target customer owned bot

Non-blocking advisory warning when source mannequin doesn't end in [bot]

Reclaim fails against bot owned by another org

[Copilot]

🟡 Changes recommended

Case-insensitive bot handling bypasses the TUI’s irreversible-operation warning, and confirmation can understate the number of affected mannequins.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds bot-account targets to mannequin reclamation, including irreversible-operation safeguards and failure handling.

Changes:

• Adds bot reclaim confirmation and warning flows.
• Makes bot detection case-insensitive and failures actionable.
• Updates documentation and tests.

File summaries

File	Description
README.md	Documents bot reclamation.
internal/ghapi/reclaim.go	Updates bot detection and failure handling.
internal/ghapi/reclaim_test.go	Tests bot failure propagation.
internal/ghapi/ghapi.go	Clarifies feature-flag documentation.
internal/ghapi/ghapi_test.go	Modernizes API assertions.
internal/cmd/target/mannequins.go	Adds bot confirmation prompts.
internal/cmd/target/mannequins_test.go	Tests CLI bot workflows.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 3
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Confirmation handling can misclassify normalized bot targets, omit TUI warnings, and reject or double-prompt valid bot reclaims.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

internal/cmd/target/mannequins.go:249

• With a bot target and --skip-invitation, this prompt runs after ensureSkipInvitationAllowed has already enforced org-admin membership and consumed a confirmation. Bot reattribution does not use that flag and also permits enterprise admins, so this combination can reject an otherwise authorized admin or unexpectedly prompt twice. Bypass skip-invitation validation for bot-only single reclaims (or reject the incompatible flag explicitly).

			if err := confirmBotReclaims(cmd, log, []ghapi.MannequinRecord{{MannequinUser: mannequinUser, TargetUser: targetUser}}, noPrompt); err != nil {

• Files reviewed: 9/9 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Blank CSV targets are misclassified as user reclaims, causing inaccurate prompts and potentially invalid authorization checks.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 1
• Review effort level: Balanced

[isaevt]

Left a question about internal/ghapi/reclaim.go

Also, the PR’s “Testing/screenshots” section still says “To be filled in.” It would be useful to record the manual validation performed for this irreversible operation.

[isaevt]

When a bot target cannot be resolved, could we return an error instead of skipping the row?

With the current continue, a CSV can partially complete: earlier bot rows may be reattributed, a misspelled [bot] target is skipped, and the command still exits successfully. That makes it hard for automation to detect an incomplete reclaim.

[iomekam]

In my opinion, this should fail open. The log is adding the observability aspect and one failure doesn't impact the entire reclaim flow

[iomekam]

Still ramping up on this repo. LGTM

[iomekam]

I think we could simplify this by dispatching GraphQL requests based on operationName rather than strings.Contains on the query. It would make the mock less brittle if the queries change and make it clearer which operations the test supports.

Something along these lines:

type graphqlRequest struct {
	OperationName string `json:"operationName"`
}

srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	var req graphqlRequest
	require.NoError(t, json.NewDecoder(r.Body).Decode(&req))

	switch req.OperationName {
	case "GetOrganization":
		writeJSON(w, `{"data":{"organization":{"id":"ORG"}}}`)
	case "ListMannequins":
		writeJSON(w, `...`)
	case "GetUser":
		writeJSON(w, `{"data":{"user":{"id":"u2"}}}`)
	case "ReattributeMannequinToBot":
		*botCalled = true
		writeJSON(w, `...`)
	case "CreateAttributionInvitation":
		*invited = true
		writeJSON(w, `...`)
	default:
		require.Failf(t, "unexpected GraphQL operation", "operation: %q", req.OperationName)
	}
}))

[dpmex4527]

Good suggestion. Fixed this in dbea179

[iomekam]

Similar thought here. Rather than having the handler inspect the query with strings.Contains, I’d consider mapping the expected GQL queries to their responses and having the handler just look up the request query.

For example:

func botClaimServer(t *testing.T, mannequinLogin string) (*httptest.Server, *bool) {
	const (
		mannequinID = "m1"
		botLogin    = "example-ci[bot]"
		botNodeID   = "BOT1"
	)

	called := new(bool)

	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if strings.HasPrefix(r.URL.Path, "/users/") {
			_, _ = fmt.Fprintf(w, `{"type":"Bot","node_id":%q}`, botNodeID)
			return
		}

		var req struct {
			OperationName string `json:"operationName"`
		}
		require.NoError(t, json.NewDecoder(r.Body).Decode(&req))

		w.Header().Set("Content-Type", "application/json")

		switch req.OperationName {
		case "GetOrganization":
			_, _ = io.WriteString(w, `{"data":{"organization":{"id":"ORG"}}}`)

		case "ListMannequins":
			_, _ = fmt.Fprintf(w,
				`{"data":{"node":{"mannequins":{"pageInfo":{"endCursor":"","hasNextPage":false},"nodes":[{"id":%q,"login":%q,"claimant":null}]}}}}`,
				mannequinID, mannequinLogin,
			)

		case "ReattributeMannequinToBot":
			*called = true
			_, _ = fmt.Fprintf(w,
				`{"data":{"reattributeMannequinToBot":{"source":{"id":%q,"login":%q},"target":{"id":%q,"login":%q}}}}`,
				mannequinID, mannequinLogin, botNodeID, botLogin,
			)

		default:
			require.Failf(t, "unexpected GraphQL operation", "operation: %q", req.OperationName)
		}
	}))

	return srv, called
}

I think this is preferable to strings.Contains(req.Query, ...) because the mock doesn't care about the formatting or structure of the query. It also makes it immediately obvious which GraphQL operations the test expects.

[dpmex4527]

Fixed this in dbea179

[iomekam]

In my opinion, this should fail open. The log is adding the observability aspect and one failure doesn't impact the entire reclaim flow