fix(mcp): code review I2 — admin expires_at must be future + Part 3 tx TODO + scope JSON round-trip tests

- IssueAdmin now rejects past/now ExpiresAt with CodeMcpTokenExpiresRequired
- TestIssueAdmin_ExpiresMustBeFuture covers the new path
- IssueSystemToken doc comment notes pgRepo currently ignores ctx tx; Part 3
  must add Repository.WithTx for true same-transaction guarantee with
  acp_sessions (spec §6.1)
- New scope_test.go (whitebox) verifies JSON round-trip preserves nil-vs-empty
  semantics on Tools / ProjectIDs and exercises AllowsTool / AllowsProject
  edges that Authenticate / future tool gating depend on

internal/mcp/scope_test.go

@@ -0,0 +1,83 @@
+package mcp
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// Scope 的 JSON round-trip 必须保留 nil-vs-empty 语义：
+//   - nil Tools/ProjectIDs = "允许全部"
+//   - empty slice          = "全部禁止"
+// repository.go 的 scopeToJSON / scopeFromJSON 是 encoding/json 的薄封装，
+// 这里直接测公开类型的 marshal/unmarshal 路径，不耦合具体 helper 实现。
+
+func TestScope_JSON_RoundTrip_Empty(t *testing.T) {
+	t.Parallel()
+
+	raw, err := json.Marshal(Scope{})
+	require.NoError(t, err)
+	assert.JSONEq(t, "{}", string(raw), "empty Scope should marshal to {} via omitempty")
+
+	var got Scope
+	require.NoError(t, json.Unmarshal(raw, &got))
+	assert.Nil(t, got.Tools, "Tools must remain nil — meaning 'allow all'")
+	assert.Nil(t, got.ProjectIDs, "ProjectIDs must remain nil — meaning 'allow all'")
+}
+
+func TestScope_JSON_RoundTrip_Populated(t *testing.T) {
+	t.Parallel()
+
+	pid := uuid.New()
+	in := Scope{
+		Tools:      []string{"list_issues", "create_issue"},
+		ProjectIDs: []uuid.UUID{pid},
+	}
+	raw, err := json.Marshal(in)
+	require.NoError(t, err)
+
+	var out Scope
+	require.NoError(t, json.Unmarshal(raw, &out))
+	assert.Equal(t, in.Tools, out.Tools)
+	assert.Equal(t, in.ProjectIDs, out.ProjectIDs)
+}
+
+func TestScope_NilVsEmpty_AllowsTool(t *testing.T) {
+	t.Parallel()
+
+	var nilScope *Scope
+	assert.True(t, nilScope.AllowsTool("anything"), "nil receiver allows all")
+
+	zero := &Scope{} // Tools/ProjectIDs both nil
+	assert.True(t, zero.AllowsTool("anything"), "nil Tools allows all")
+
+	empty := &Scope{Tools: []string{}}
+	assert.False(t, empty.AllowsTool("anything"), "empty Tools forbids all")
+
+	allow := &Scope{Tools: []string{"list_issues"}}
+	assert.True(t, allow.AllowsTool("list_issues"))
+	assert.False(t, allow.AllowsTool("create_issue"))
+}
+
+func TestScope_NilVsEmpty_AllowsProject(t *testing.T) {
+	t.Parallel()
+
+	pidA := uuid.New()
+	pidB := uuid.New()
+
+	var nilScope *Scope
+	assert.True(t, nilScope.AllowsProject(pidA))
+
+	zero := &Scope{}
+	assert.True(t, zero.AllowsProject(pidA), "nil ProjectIDs allows all")
+
+	empty := &Scope{ProjectIDs: []uuid.UUID{}}
+	assert.False(t, empty.AllowsProject(pidA), "empty ProjectIDs forbids all")
+
+	allow := &Scope{ProjectIDs: []uuid.UUID{pidA}}
+	assert.True(t, allow.AllowsProject(pidA))
+	assert.False(t, allow.AllowsProject(pidB))
+}

internal/mcp/token_service.go

@@ -79,6 +79,9 @@ func (s *tokenService) IssueAdmin(ctx context.Context, in IssueAdminInput) (Issu
 	if in.ExpiresAt.IsZero() {
 		return IssueResult{}, errs.New(errs.CodeMcpTokenExpiresRequired, "expires_at is required for admin token")
 	}
+	if !in.ExpiresAt.After(time.Now()) {
+		return IssueResult{}, errs.New(errs.CodeMcpTokenExpiresRequired, "expires_at must be in the future")
+	}
 
 	plain, hash, err := NewToken()
 	if err != nil {
@@ -128,6 +131,12 @@ func (s *tokenService) IssueAdmin(ctx context.Context, in IssueAdminInput) (Issu
 // FK 校验失败。spec §6.1 明确建议同事务提交。本 service 不直接管事务边界（由
 // ACP SessionService 在 Create 流程内编排），但调用顺序由调用方保证。
 //
+// TODO(Part 3): 当前 pgRepo 持有 *pgxpool.Pool 并忽略 ctx 中可能存在的 pgx tx，
+// 因此严格意义上 IssueSystemToken 与 acp_sessions row 的写入并不在同一事务里 ——
+// 调用顺序虽然能避开 FK 错误，但不是真正的原子提交。要兑现 spec §6.1 同事务
+// 保证，Part 3 需要为 Repository 增加 `WithTx(tx pgx.Tx) Repository` 构造，
+// 并在 acp.SessionService.Create 中以同一事务执行 acp_sessions 插入与本次签发。
+//
 // 默认 ExpiresIn = 24h；默认 Tools 为内置 PM 工具白名单（spec §6.2）。
 // 写一条 audit `mcp.token.create_system`。
 func (s *tokenService) IssueSystemToken(ctx context.Context, in IssueSystemTokenInput) (IssueResult, error) {

internal/mcp/token_service_test.go

@@ -196,6 +196,21 @@ func TestIssueAdmin_ExpiresRequired(t *testing.T) {
 	assert.Equal(t, errs.CodeMcpTokenExpiresRequired, ae.Code)
 }
 
+func TestIssueAdmin_ExpiresMustBeFuture(t *testing.T) {
+	t.Parallel()
+	svc := mcp.NewTokenService(newFakeRepo(), &fakeAudit{})
+
+	_, err := svc.IssueAdmin(context.Background(), mcp.IssueAdminInput{
+		UserID:    uuid.New(),
+		Name:      "x",
+		ExpiresAt: time.Now().Add(-time.Hour), // past
+		CreatedBy: uuid.New(),
+	})
+	ae, ok := errs.As(err)
+	require.True(t, ok)
+	assert.Equal(t, errs.CodeMcpTokenExpiresRequired, ae.Code)
+}
+
 func TestIssueSystemToken_Defaults(t *testing.T) {
 	t.Parallel()
 	repo := newFakeRepo()