Skip to content

fix: return ActionRequiredResult for org invites instead of SuccessResult#139

Open
highb wants to merge 1 commit intomainfrom
highb/iga-1212-github-invite-action-required
Open

fix: return ActionRequiredResult for org invites instead of SuccessResult#139
highb wants to merge 1 commit intomainfrom
highb/iga-1212-github-invite-action-required

Conversation

@highb
Copy link
Copy Markdown

@highb highb commented Apr 7, 2026

Summary

  • CreateAccount now returns ActionRequiredResult (not SuccessResult) when sending a GitHub org invite, since the user must accept the invitation before they're an org member
  • Returns AlreadyExistsResult when the user is already an org member (422 "already a member")
  • Returns ActionRequiredResult when a pending invite already exists (422 "already invited")

Fixes the root cause of downstream team membership grants failing with "only users can be granted team membership" — the C1 engine was treating the invite as a completed account creation.

Closes: IGA-1311 baton-github: Return ActionRequiredResult for org invites

Test plan

  • go build ./... passes
  • go vet ./... passes
  • Verify new invite returns ActionRequiredResult with resource and message
  • Verify inviting an existing member returns AlreadyExistsResult
  • Verify re-inviting a pending user returns ActionRequiredResult with pending message
  • Confirm C1 engine handles ActionRequiredResult correctly (behind feature flag, tracked in IGA-1212)

🤖 Generated with Claude Code

…sult

CreateAccount was returning SuccessResult when sending a GitHub org
invite, but the user hasn't actually joined yet — they must accept the
invitation first. This caused downstream team membership grants to fail.

Now returns the correct response type for each case:
- New invite sent: ActionRequiredResult (pending acceptance)
- Already an org member: AlreadyExistsResult
- Already has pending invite: ActionRequiredResult

Ref: IGA-1311

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@highb highb requested a review from a team April 7, 2026 21:20
@linear
Copy link
Copy Markdown

linear bot commented Apr 7, 2026

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

Claude finished @highb's task in 1m 4s —— View job


PR Review: Return ActionRequiredResult for org invites

  • Read changed files and gather context
  • Analyze the diff against origin/main
  • Post review feedback

Overall: This is a clean, well-scoped fix. The change correctly addresses the semantic mismatch where SuccessResult was being returned for an operation (org invite) that isn't actually complete until the user accepts.

What changed

  1. Success path (invitation.go:169-172): SuccessResultActionRequiredResult with a clear message and the invitation resource attached.
  2. Already-a-member error (invitation.go:146-148): Returns AlreadyExistsResult on 422 "already a member".
  3. Already-invited error (invitation.go:149-153): Returns ActionRequiredResult with a pending message on 422 "already invited".
  4. Two new helper functions (isAlreadyOrgMemberError, isAlreadyInvitedError): Parse GitHub ErrorResponse for specific 422 messages.

Feedback

Looks good — no blocking issues. A few observations:

  1. Error matching is reasonably defensive. Checking both ghErr.Message and ghErr.Errors[].Message with case-insensitive matching is good practice for GitHub API error parsing. The string patterns ("already a member", "already invited", "already been invited") cover the known error messages.

  2. AlreadyExistsResult without a Resource (invitation.go:147): The AlreadyExistsResult struct has a Resource field, but it's left nil here. This is fine since we don't have the existing member's resource from the error response — just noting it for awareness in case the C1 engine ever requires a resource on AlreadyExistsResult.

  3. ActionRequiredResult for already-invited (invitation.go:150-152): No Resource is set here either. This differs from the success path which attaches the invitation resource. This makes sense since we don't get the existing invitation back from the 422 error, but worth confirming the C1 engine handles a nil Resource on ActionRequiredResult gracefully.

  4. Idempotency is solid. Both 422 cases return nil error per the CLAUDE.md pattern for grant idempotency, which is correct — these are expected states, not failures.

  5. Messages are clear and actionable. Both "User must accept the existing invitation" and "User must accept the invitation before team membership can be granted" give good context for operators.

LGTM — this is a well-targeted fix for the root cause described in the PR summary. The C1 engine will no longer treat an invitation as a completed account creation, which should resolve the downstream team membership grant failures.

})
if err != nil {
if isAlreadyOrgMemberError(err, resp) {
return &v2.CreateAccountResponse_AlreadyExistsResult{}, nil, nil, nil
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to return the resource as part of the response so c1 caches this value?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants