refactor(guild): introduce service layer for guild member operations

Extract business logic from handleMsgMhfOperateGuildMember into
GuildService.OperateMember, establishing the handler→service→repo
layering pattern. The handler is now ~20 lines of protocol glue
(type-assert, map action, call service, send ACK, notify).

GuildService owns authorization checks, repo coordination, mail
composition, and best-effort mail delivery. It accepts plain Go
types (no mhfpacket or Session imports), making it fully testable
with mock repos. Cross-channel notification stays in the handler
since it requires Session.

Adds 7 table-driven service-level tests covering accept/reject/kick,
authorization, repo errors, mail errors, and unknown actions.

server/channelserver/handlers_guild_ops.go

@@ -266,58 +266,32 @@ func handleAvoidLeadershipUpdate(s *Session, pkt *mhfpacket.MsgMhfOperateGuild,
 func handleMsgMhfOperateGuildMember(s *Session, p mhfpacket.MHFPacket) {
 	pkt := p.(*mhfpacket.MsgMhfOperateGuildMember)
 
-	guild, err := s.server.guildRepo.GetByCharID(pkt.CharID)
-
-	if err != nil || guild == nil {
-		doAckSimpleFail(s, pkt.AckHandle, make([]byte, 4))
-		return
-	}
-
-	actorCharacter, err := s.server.guildRepo.GetCharacterMembership(s.charID)
-
-	if err != nil || (!actorCharacter.IsSubLeader() && guild.LeaderCharID != s.charID) {
-		doAckSimpleFail(s, pkt.AckHandle, make([]byte, 4))
-		return
-	}
-
-	var mail Mail
-	switch pkt.Action {
-	case mhfpacket.OPERATE_GUILD_MEMBER_ACTION_ACCEPT:
-		err = s.server.guildRepo.AcceptApplication(guild.ID, pkt.CharID)
-		mail = Mail{
-			RecipientID:     pkt.CharID,
-			Subject:         "Accepted!",
-			Body:            fmt.Sprintf("Your application to join 「%s」 was accepted.", guild.Name),
-			IsSystemMessage: true,
-		}
-	case mhfpacket.OPERATE_GUILD_MEMBER_ACTION_REJECT:
-		err = s.server.guildRepo.RejectApplication(guild.ID, pkt.CharID)
-		mail = Mail{
-			RecipientID:     pkt.CharID,
-			Subject:         "Rejected",
-			Body:            fmt.Sprintf("Your application to join 「%s」 was rejected.", guild.Name),
-			IsSystemMessage: true,
-		}
-	case mhfpacket.OPERATE_GUILD_MEMBER_ACTION_KICK:
-		err = s.server.guildRepo.RemoveCharacter(pkt.CharID)
-		mail = Mail{
-			RecipientID:     pkt.CharID,
-			Subject:         "Kicked",
-			Body:            fmt.Sprintf("You were kicked from 「%s」.", guild.Name),
-			IsSystemMessage: true,
-		}
-	default:
-		doAckSimpleFail(s, pkt.AckHandle, make([]byte, 4))
+	action, ok := mapMemberAction(pkt.Action)
+	if !ok {
 		s.logger.Warn("Unhandled operateGuildMember action", zap.Uint8("action", pkt.Action))
+		doAckSimpleFail(s, pkt.AckHandle, make([]byte, 4))
+		return
 	}
 
+	result, err := s.server.guildService.OperateMember(s.charID, pkt.CharID, action)
 	if err != nil {
 		doAckSimpleFail(s, pkt.AckHandle, make([]byte, 4))
-	} else {
-		if err := s.server.mailRepo.SendMail(mail.SenderID, mail.RecipientID, mail.Subject, mail.Body, 0, 0, false, true); err != nil {
-			s.logger.Warn("Failed to send guild member operation mail", zap.Error(err))
+		return
 	}
-		s.server.Registry.NotifyMailToCharID(pkt.CharID, s, &mail)
+
+	s.server.Registry.NotifyMailToCharID(result.MailRecipientID, s, &result.Mail)
 	doAckSimpleSucceed(s, pkt.AckHandle, make([]byte, 4))
 }
+
+func mapMemberAction(proto uint8) (GuildMemberAction, bool) {
+	switch proto {
+	case mhfpacket.OPERATE_GUILD_MEMBER_ACTION_ACCEPT:
+		return GuildMemberActionAccept, true
+	case mhfpacket.OPERATE_GUILD_MEMBER_ACTION_REJECT:
+		return GuildMemberActionReject, true
+	case mhfpacket.OPERATE_GUILD_MEMBER_ACTION_KICK:
+		return GuildMemberActionKick, true
+	default:
+		return 0, false
+	}
 }

server/channelserver/handlers_guild_ops_test.go

@@ -448,6 +448,7 @@ func TestOperateGuildMember_Accept(t *testing.T) {
 	guildMock.guild.LeaderCharID = 1
 	server.guildRepo = guildMock
 	server.mailRepo = mailMock
+	ensureGuildService(server)
 	session := createMockSession(1, server)
 
 	pkt := &mhfpacket.MsgMhfOperateGuildMember{
@@ -486,6 +487,7 @@ func TestOperateGuildMember_Reject(t *testing.T) {
 	guildMock.guild.LeaderCharID = 1
 	server.guildRepo = guildMock
 	server.mailRepo = mailMock
+	ensureGuildService(server)
 	session := createMockSession(1, server)
 
 	pkt := &mhfpacket.MsgMhfOperateGuildMember{
@@ -515,6 +517,7 @@ func TestOperateGuildMember_Kick(t *testing.T) {
 	guildMock.guild.LeaderCharID = 1
 	server.guildRepo = guildMock
 	server.mailRepo = mailMock
+	ensureGuildService(server)
 	session := createMockSession(1, server)
 
 	pkt := &mhfpacket.MsgMhfOperateGuildMember{
@@ -544,6 +547,7 @@ func TestOperateGuildMember_MailError(t *testing.T) {
 	guildMock.guild.LeaderCharID = 1
 	server.guildRepo = guildMock
 	server.mailRepo = mailMock
+	ensureGuildService(server)
 	session := createMockSession(1, server)
 
 	pkt := &mhfpacket.MsgMhfOperateGuildMember{
@@ -572,6 +576,7 @@ func TestOperateGuildMember_NotLeaderOrSub(t *testing.T) {
 	guildMock.guild.LeaderCharID = 999 // not the session char
 	server.guildRepo = guildMock
 	server.mailRepo = &mockMailRepo{}
+	ensureGuildService(server)
 	session := createMockSession(1, server)
 
 	pkt := &mhfpacket.MsgMhfOperateGuildMember{

server/channelserver/svc_guild.go

@@ -0,0 +1,106 @@
+package channelserver
+
+import (
+	"errors"
+	"fmt"
+
+	"go.uber.org/zap"
+)
+
+// GuildMemberAction is a domain enum for guild member operations.
+type GuildMemberAction uint8
+
+const (
+	GuildMemberActionAccept GuildMemberAction = iota + 1
+	GuildMemberActionReject
+	GuildMemberActionKick
+)
+
+// ErrUnauthorized is returned when the actor lacks permission for the operation.
+var ErrUnauthorized = errors.New("unauthorized")
+
+// ErrUnknownAction is returned for unrecognized guild member actions.
+var ErrUnknownAction = errors.New("unknown guild member action")
+
+// OperateMemberResult holds the outcome of a guild member operation.
+type OperateMemberResult struct {
+	MailRecipientID uint32
+	Mail            Mail
+}
+
+// GuildService encapsulates guild business logic, sitting between handlers and repos.
+type GuildService struct {
+	guildRepo GuildRepo
+	mailRepo  MailRepo
+	charRepo  CharacterRepo
+	logger    *zap.Logger
+}
+
+// NewGuildService creates a new GuildService.
+func NewGuildService(gr GuildRepo, mr MailRepo, cr CharacterRepo, log *zap.Logger) *GuildService {
+	return &GuildService{
+		guildRepo: gr,
+		mailRepo:  mr,
+		charRepo:  cr,
+		logger:    log,
+	}
+}
+
+// OperateMember performs a guild member management action (accept/reject/kick).
+// The actor must be the guild leader or a sub-leader. On success, a notification
+// mail is sent (best-effort) and the result is returned for protocol-level notification.
+func (svc *GuildService) OperateMember(actorCharID, targetCharID uint32, action GuildMemberAction) (*OperateMemberResult, error) {
+	guild, err := svc.guildRepo.GetByCharID(targetCharID)
+	if err != nil || guild == nil {
+		return nil, fmt.Errorf("guild lookup for char %d: %w", targetCharID, err)
+	}
+
+	actorMember, err := svc.guildRepo.GetCharacterMembership(actorCharID)
+	if err != nil || (!actorMember.IsSubLeader() && guild.LeaderCharID != actorCharID) {
+		return nil, ErrUnauthorized
+	}
+
+	var mail Mail
+	switch action {
+	case GuildMemberActionAccept:
+		err = svc.guildRepo.AcceptApplication(guild.ID, targetCharID)
+		mail = Mail{
+			RecipientID:     targetCharID,
+			Subject:         "Accepted!",
+			Body:            fmt.Sprintf("Your application to join 「%s」 was accepted.", guild.Name),
+			IsSystemMessage: true,
+		}
+	case GuildMemberActionReject:
+		err = svc.guildRepo.RejectApplication(guild.ID, targetCharID)
+		mail = Mail{
+			RecipientID:     targetCharID,
+			Subject:         "Rejected",
+			Body:            fmt.Sprintf("Your application to join 「%s」 was rejected.", guild.Name),
+			IsSystemMessage: true,
+		}
+	case GuildMemberActionKick:
+		err = svc.guildRepo.RemoveCharacter(targetCharID)
+		mail = Mail{
+			RecipientID:     targetCharID,
+			Subject:         "Kicked",
+			Body:            fmt.Sprintf("You were kicked from 「%s」.", guild.Name),
+			IsSystemMessage: true,
+		}
+	default:
+		return nil, ErrUnknownAction
+	}
+
+	if err != nil {
+		return nil, fmt.Errorf("guild member action %d: %w", action, err)
+	}
+
+	// Send mail best-effort
+	if mailErr := svc.mailRepo.SendMail(mail.SenderID, mail.RecipientID, mail.Subject, mail.Body, 0, 0, false, true); mailErr != nil {
+		svc.logger.Warn("Failed to send guild member operation mail", zap.Error(mailErr))
+	}
+
+	return &OperateMemberResult{
+		MailRecipientID: targetCharID,
+		Mail:            mail,
+	}, nil
+}

server/channelserver/svc_guild_test.go

@@ -0,0 +1,170 @@
+package channelserver
+
+import (
+	"errors"
+	"testing"
+
+	"go.uber.org/zap"
+)
+
+func newTestGuildService(gr GuildRepo, mr MailRepo) *GuildService {
+	logger, _ := zap.NewDevelopment()
+	return NewGuildService(gr, mr, nil, logger)
+}
+
+func TestGuildService_OperateMember(t *testing.T) {
+	tests := []struct {
+		name          string
+		actorCharID   uint32
+		targetCharID  uint32
+		action        GuildMemberAction
+		guild         *Guild
+		membership    *GuildMember
+		acceptErr     error
+		rejectErr     error
+		removeErr     error
+		sendErr       error
+		wantErr       bool
+		wantErrIs     error
+		wantAccepted  uint32
+		wantRejected  uint32
+		wantRemoved   uint32
+		wantMailCount int
+		wantRecipient uint32
+		wantMailSubj  string
+	}{
+		{
+			name:          "accept application as leader",
+			actorCharID:   1,
+			targetCharID:  42,
+			action:        GuildMemberActionAccept,
+			guild:         &Guild{ID: 10, Name: "TestGuild", GuildLeader: GuildLeader{LeaderCharID: 1}},
+			membership:    &GuildMember{GuildID: 10, CharID: 1, IsLeader: true, OrderIndex: 1},
+			wantAccepted:  42,
+			wantMailCount: 1,
+			wantRecipient: 42,
+			wantMailSubj:  "Accepted!",
+		},
+		{
+			name:          "reject application as sub-leader",
+			actorCharID:   2,
+			targetCharID:  42,
+			action:        GuildMemberActionReject,
+			guild:         &Guild{ID: 10, Name: "TestGuild", GuildLeader: GuildLeader{LeaderCharID: 1}},
+			membership:    &GuildMember{GuildID: 10, CharID: 2, OrderIndex: 2}, // sub-leader
+			wantRejected:  42,
+			wantMailCount: 1,
+			wantRecipient: 42,
+			wantMailSubj:  "Rejected",
+		},
+		{
+			name:          "kick member as leader",
+			actorCharID:   1,
+			targetCharID:  42,
+			action:        GuildMemberActionKick,
+			guild:         &Guild{ID: 10, Name: "TestGuild", GuildLeader: GuildLeader{LeaderCharID: 1}},
+			membership:    &GuildMember{GuildID: 10, CharID: 1, IsLeader: true, OrderIndex: 1},
+			wantRemoved:   42,
+			wantMailCount: 1,
+			wantRecipient: 42,
+			wantMailSubj:  "Kicked",
+		},
+		{
+			name:         "unauthorized - not leader or sub",
+			actorCharID:  5,
+			targetCharID: 42,
+			action:       GuildMemberActionAccept,
+			guild:        &Guild{ID: 10, Name: "TestGuild", GuildLeader: GuildLeader{LeaderCharID: 1}},
+			membership:   &GuildMember{GuildID: 10, CharID: 5, OrderIndex: 10},
+			wantErr:      true,
+			wantErrIs:    ErrUnauthorized,
+		},
+		{
+			name:         "repo error on accept",
+			actorCharID:  1,
+			targetCharID: 42,
+			action:       GuildMemberActionAccept,
+			guild:        &Guild{ID: 10, Name: "TestGuild", GuildLeader: GuildLeader{LeaderCharID: 1}},
+			membership:   &GuildMember{GuildID: 10, CharID: 1, IsLeader: true, OrderIndex: 1},
+			acceptErr:    errors.New("db error"),
+			wantErr:      true,
+		},
+		{
+			name:          "mail error is best-effort",
+			actorCharID:   1,
+			targetCharID:  42,
+			action:        GuildMemberActionAccept,
+			guild:         &Guild{ID: 10, Name: "TestGuild", GuildLeader: GuildLeader{LeaderCharID: 1}},
+			membership:    &GuildMember{GuildID: 10, CharID: 1, IsLeader: true, OrderIndex: 1},
+			sendErr:       errors.New("mail failed"),
+			wantAccepted:  42,
+			wantMailCount: 1,
+			wantRecipient: 42,
+			wantMailSubj:  "Accepted!",
+		},
+		{
+			name:         "unknown action",
+			actorCharID:  1,
+			targetCharID: 42,
+			action:       GuildMemberAction(99),
+			guild:        &Guild{ID: 10, Name: "TestGuild", GuildLeader: GuildLeader{LeaderCharID: 1}},
+			membership:   &GuildMember{GuildID: 10, CharID: 1, IsLeader: true, OrderIndex: 1},
+			wantErr:      true,
+			wantErrIs:    ErrUnknownAction,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			guildMock := &mockGuildRepoOps{
+				membership: tt.membership,
+				acceptErr:  tt.acceptErr,
+				rejectErr:  tt.rejectErr,
+				removeErr:  tt.removeErr,
+			}
+			guildMock.guild = tt.guild
+			mailMock := &mockMailRepo{sendErr: tt.sendErr}
+
+			svc := newTestGuildService(guildMock, mailMock)
+
+			result, err := svc.OperateMember(tt.actorCharID, tt.targetCharID, tt.action)
+
+			if tt.wantErr {
+				if err == nil {
+					t.Fatal("Expected error, got nil")
+				}
+				if tt.wantErrIs != nil && !errors.Is(err, tt.wantErrIs) {
+					t.Errorf("Expected error %v, got %v", tt.wantErrIs, err)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("Unexpected error: %v", err)
+			}
+
+			if tt.wantAccepted != 0 && guildMock.acceptedCharID != tt.wantAccepted {
+				t.Errorf("acceptedCharID = %d, want %d", guildMock.acceptedCharID, tt.wantAccepted)
+			}
+			if tt.wantRejected != 0 && guildMock.rejectedCharID != tt.wantRejected {
+				t.Errorf("rejectedCharID = %d, want %d", guildMock.rejectedCharID, tt.wantRejected)
+			}
+			if tt.wantRemoved != 0 && guildMock.removedCharID != tt.wantRemoved {
+				t.Errorf("removedCharID = %d, want %d", guildMock.removedCharID, tt.wantRemoved)
+			}
+			if len(mailMock.sentMails) != tt.wantMailCount {
+				t.Fatalf("sentMails count = %d, want %d", len(mailMock.sentMails), tt.wantMailCount)
+			}
+			if tt.wantMailCount > 0 {
+				if mailMock.sentMails[0].recipientID != tt.wantRecipient {
+					t.Errorf("mail recipientID = %d, want %d", mailMock.sentMails[0].recipientID, tt.wantRecipient)
+				}
+				if mailMock.sentMails[0].subject != tt.wantMailSubj {
+					t.Errorf("mail subject = %q, want %q", mailMock.sentMails[0].subject, tt.wantMailSubj)
+				}
+			}
+			if result.MailRecipientID != tt.targetCharID {
+				t.Errorf("result.MailRecipientID = %d, want %d", result.MailRecipientID, tt.targetCharID)
+			}
+		})
+	}
+}

server/channelserver/sys_channel_server.go

@@ -71,6 +71,7 @@ type Server struct {
 	miscRepo        MiscRepo
 	scenarioRepo    ScenarioRepo
 	mercenaryRepo   MercenaryRepo
+	guildService    *GuildService
 	erupeConfig     *cfg.Config
 	acceptConns     chan net.Conn
 	deleteConns     chan net.Conn
@@ -153,6 +154,8 @@ func NewServer(config *Config) *Server {
 	s.scenarioRepo = NewScenarioRepository(config.DB)
 	s.mercenaryRepo = NewMercenaryRepository(config.DB)
 
+	s.guildService = NewGuildService(s.guildRepo, s.mailRepo, s.charRepo, s.logger)
+
 	// Mezeporta
 	s.stages.Store("sl1Ns200p0a0u0", NewStage("sl1Ns200p0a0u0"))
 

server/channelserver/test_helpers_test.go

@@ -51,9 +51,16 @@ func createMockServer() *Server {
 	}
 	s.i18n = getLangStrings(s)
 	s.Registry = NewLocalChannelRegistry([]*Server{s})
+	// GuildService is wired lazily by tests that set repos then call ensureGuildService.
 	return s
 }
 
+// ensureGuildService wires the GuildService from the server's current repos.
+// Call this after setting guildRepo, mailRepo, and charRepo on the mock server.
+func ensureGuildService(s *Server) {
+	s.guildService = NewGuildService(s.guildRepo, s.mailRepo, s.charRepo, s.logger)
+}
+
 // createMockSession creates a minimal Session for testing.
 // Imported from v9.2.x-stable and adapted for main.
 func createMockSession(charID uint32, server *Server) *Session {