q792602257/agentic-coding-workflow
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(mcp): apply Part 1 review I1+I3+I4+I5

Browse Source 
- Rename MCPToken -> Token (mcp.Token reads cleaner; zero external callers yet)
- IssueSystemToken: guard against nil ACPSessionID / UserID (early input validation)
- TokenService: inject now func for clock injection (parity with RateLimiter)
- Repository CHECK constraint tests: assert wrapped CodeInternal

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Jerry Yan
2026-05-08 10:28:49 +08:00
parent 980c6fcf0e
commit a00f5a9efd
7 changed files with 106 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/mcp/token_service_test.go
+44 -18
View File
@@ -20,18 +20,18 @@ import (
type fakeRepo struct {
mu sync.Mutex
tokens map[uuid.UUID]*mcp.MCPToken
tokens map[uuid.UUID]*mcp.Token
byHash map[string]uuid.UUID
}
func newFakeRepo() *fakeRepo {
return &fakeRepo{
tokens: map[uuid.UUID]*mcp.MCPToken{},
tokens: map[uuid.UUID]*mcp.Token{},
byHash: map[string]uuid.UUID{},
}
}
func (r *fakeRepo) Create(ctx context.Context, t *mcp.MCPToken) (*mcp.MCPToken, error) {
func (r *fakeRepo) Create(ctx context.Context, t *mcp.Token) (*mcp.Token, error) {
r.mu.Lock()
defer r.mu.Unlock()
c := *t
@@ -40,7 +40,7 @@ func (r *fakeRepo) Create(ctx context.Context, t *mcp.MCPToken) (*mcp.MCPToken,
r.byHash[string(c.TokenHash)] = c.ID
return &c, nil
}
func (r *fakeRepo) GetByHash(ctx context.Context, h []byte) (*mcp.MCPToken, error) {
func (r *fakeRepo) GetByHash(ctx context.Context, h []byte) (*mcp.Token, error) {
r.mu.Lock()
defer r.mu.Unlock()
id, ok := r.byHash[string(h)]
@@ -50,7 +50,7 @@ func (r *fakeRepo) GetByHash(ctx context.Context, h []byte) (*mcp.MCPToken, erro
c := *r.tokens[id]
return &c, nil
}
func (r *fakeRepo) GetByID(ctx context.Context, id uuid.UUID) (*mcp.MCPToken, error) {
func (r *fakeRepo) GetByID(ctx context.Context, id uuid.UUID) (*mcp.Token, error) {
r.mu.Lock()
defer r.mu.Unlock()
t, ok := r.tokens[id]
@@ -60,10 +60,10 @@ func (r *fakeRepo) GetByID(ctx context.Context, id uuid.UUID) (*mcp.MCPToken, er
c := *t
return &c, nil
}
func (r *fakeRepo) List(ctx context.Context, caller *uuid.UUID, activeOnly bool) ([]*mcp.MCPToken, error) {
func (r *fakeRepo) List(ctx context.Context, caller *uuid.UUID, activeOnly bool) ([]*mcp.Token, error) {
r.mu.Lock()
defer r.mu.Unlock()
var out []*mcp.MCPToken
var out []*mcp.Token
for _, t := range r.tokens {
if caller != nil && (t.UserID != *caller || t.Issuer != mcp.IssuerAdmin) {
continue
@@ -151,7 +151,7 @@ func TestIssueAdmin_HappyPath(t *testing.T) {
t.Parallel()
repo := newFakeRepo()
au := &fakeAudit{}
svc := mcp.NewTokenService(repo, au)
svc := mcp.NewTokenService(repo, au, nil)
uid := uuid.New()
in := mcp.IssueAdminInput{
@@ -169,7 +169,7 @@ func TestIssueAdmin_HappyPath(t *testing.T) {
func TestIssueAdmin_NameRequired(t *testing.T) {
t.Parallel()
svc := mcp.NewTokenService(newFakeRepo(), &fakeAudit{})
svc := mcp.NewTokenService(newFakeRepo(), &fakeAudit{}, nil)
_, err := svc.IssueAdmin(context.Background(), mcp.IssueAdminInput{
UserID: uuid.New(),
@@ -184,7 +184,7 @@ func TestIssueAdmin_NameRequired(t *testing.T) {
func TestIssueAdmin_ExpiresRequired(t *testing.T) {
t.Parallel()
svc := mcp.NewTokenService(newFakeRepo(), &fakeAudit{})
svc := mcp.NewTokenService(newFakeRepo(), &fakeAudit{}, nil)
_, err := svc.IssueAdmin(context.Background(), mcp.IssueAdminInput{
UserID: uuid.New(),
@@ -198,7 +198,7 @@ func TestIssueAdmin_ExpiresRequired(t *testing.T) {
func TestIssueAdmin_ExpiresMustBeFuture(t *testing.T) {
t.Parallel()
svc := mcp.NewTokenService(newFakeRepo(), &fakeAudit{})
svc := mcp.NewTokenService(newFakeRepo(), &fakeAudit{}, nil)
_, err := svc.IssueAdmin(context.Background(), mcp.IssueAdminInput{
UserID: uuid.New(),
@@ -215,7 +215,7 @@ func TestIssueSystemToken_Defaults(t *testing.T) {
t.Parallel()
repo := newFakeRepo()
au := &fakeAudit{}
svc := mcp.NewTokenService(repo, au)
svc := mcp.NewTokenService(repo, au, nil)
uid := uuid.New()
sid := uuid.New()
@@ -243,7 +243,7 @@ func TestIssueSystemToken_Defaults(t *testing.T) {
func TestAuthenticate_HappyPath(t *testing.T) {
t.Parallel()
repo := newFakeRepo()
svc := mcp.NewTokenService(repo, &fakeAudit{})
svc := mcp.NewTokenService(repo, &fakeAudit{}, nil)
res, err := svc.IssueAdmin(context.Background(), mcp.IssueAdminInput{
UserID: uuid.New(),
@@ -260,7 +260,7 @@ func TestAuthenticate_HappyPath(t *testing.T) {
func TestAuthenticate_Empty(t *testing.T) {
t.Parallel()
svc := mcp.NewTokenService(newFakeRepo(), &fakeAudit{})
svc := mcp.NewTokenService(newFakeRepo(), &fakeAudit{}, nil)
_, err := svc.Authenticate(context.Background(), "")
ae, ok := errs.As(err)
@@ -271,7 +271,7 @@ func TestAuthenticate_Empty(t *testing.T) {
func TestAuthenticate_Revoked(t *testing.T) {
t.Parallel()
repo := newFakeRepo()
svc := mcp.NewTokenService(repo, &fakeAudit{})
svc := mcp.NewTokenService(repo, &fakeAudit{}, nil)
res, _ := svc.IssueAdmin(context.Background(), mcp.IssueAdminInput{
UserID: uuid.New(),
@@ -290,7 +290,7 @@ func TestAuthenticate_Revoked(t *testing.T) {
func TestAuthenticate_Expired(t *testing.T) {
t.Parallel()
repo := newFakeRepo()
svc := mcp.NewTokenService(repo, &fakeAudit{})
svc := mcp.NewTokenService(repo, &fakeAudit{}, nil)
res, _ := svc.IssueAdmin(context.Background(), mcp.IssueAdminInput{
UserID: uuid.New(),
@@ -310,7 +310,7 @@ func TestAuthenticate_Expired(t *testing.T) {
func TestAuthenticate_TamperedToken(t *testing.T) {
t.Parallel()
svc := mcp.NewTokenService(newFakeRepo(), &fakeAudit{})
svc := mcp.NewTokenService(newFakeRepo(), &fakeAudit{}, nil)
_, err := svc.Authenticate(context.Background(), "mcpt_tampered_value")
ae, ok := errs.As(err)
@@ -322,7 +322,7 @@ func TestRevokeBySession_AuditPerToken(t *testing.T) {
t.Parallel()
repo := newFakeRepo()
au := &fakeAudit{}
svc := mcp.NewTokenService(repo, au)
svc := mcp.NewTokenService(repo, au, nil)
uid := uuid.New()
sid := uuid.New()
@@ -352,5 +352,31 @@ func TestRevokeBySession_AuditPerToken(t *testing.T) {
assert.Equal(t, 3, revokeCnt)
}
func TestIssueSystemToken_NilACPSessionID(t *testing.T) {
t.Parallel()
svc := mcp.NewTokenService(newFakeRepo(), &fakeAudit{}, nil)
_, err := svc.IssueSystemToken(context.Background(), mcp.IssueSystemTokenInput{
UserID: uuid.New(),
ACPSessionID: uuid.Nil,
})
ae, ok := errs.As(err)
require.True(t, ok)
assert.Equal(t, errs.CodeInvalidInput, ae.Code)
}
func TestIssueSystemToken_NilUserID(t *testing.T) {
t.Parallel()
svc := mcp.NewTokenService(newFakeRepo(), &fakeAudit{}, nil)
_, err := svc.IssueSystemToken(context.Background(), mcp.IssueSystemTokenInput{
UserID: uuid.Nil,
ACPSessionID: uuid.New(),
})
ae, ok := errs.As(err)
require.True(t, ok)
assert.Equal(t, errs.CodeInvalidInput, ae.Code)
}
// 编译期保证：类型实现接口
var _ = errors.New