diff --git a/server/internal/repo/markread_test.go b/server/internal/repo/markread_test.go index 898589e..d447953 100644 --- a/server/internal/repo/markread_test.go +++ b/server/internal/repo/markread_test.go @@ -2,6 +2,7 @@ package repo import ( "context" + "strings" "testing" "github.com/agentmail/gateway/internal/db" @@ -142,3 +143,94 @@ func TestMarkAllInboxReadForSkipsArchivedAndOthers(t *testing.T) { t.Fatal("别人的邮件被标掉了") } } + +/* +★★ 2026-10-01:权限决策的已读记账,decider 为空时**静默失效**。 + +# 形状 + +`DecidePermission` 的函数头承诺「权限邮件对他(决策人)也应当变成已读」。 +但那两段记账都被 `if decider != ""` 包着,而**未读判据是 mail_reads** +(`unreadFor` / `readStateFor`),不是行级那列。⇒ decider 为空时: + + mails.status = 'read' (全局冗余列改了) + mail_reads —— 没有这一行 (事实表没记) + + ⇒ 收件人在收件箱里**永远看到这封未读**,尽管它早已决策完(同意/拒绝)。 + +# 线上实证 + +13 封这样的历史邮件,收件人均为 jianf,2026-09-08 / 09-13,主题全是 +「权限请求: 请求执行 bash」—— 每一次都已被人类决策过。 + +# 为什么 HTTP 路径今天触发不了 + +`permission.go` 会沿会话树上溯找人类(`NearestHumanInThread`), +且 HTTP 层传的是 `user.Username`。但 repo 层是**所有调用方的门**, +`MarkMailRead` 早就为同一件事挡了(reader 必填),只挡它一个等于漏网。 + +# 方向 + +保守:报错让调用方看到「决策没落库」; +静默放行让收件人反复看到一封已决策完的权限询问。 +*/ + +func TestDecidePermissionRejectsEmptyDecider(t *testing.T) { + setupTestDB(t) + ctx := context.Background() + + _, err := DecidePermission(ctx, uuid.New(), " ", "同意") + if err == nil { + t.Fatal("★ 空 decider 必须报错(否则这封权限邮件在收件人收件箱里永远显示未读)") + } + if !strings.Contains(err.Error(), "decider") { + t.Fatalf("错误文案要点明是 decider 的问题,实际: %v", err) + } +} + +// 回归判据:**改 mails.status 就必须记 mail_reads**,两者不能只改其一。 +// 变异(把 decider 守卫去掉 / 改成静默跳过)后本格变红。 +func TestDecidePermissionAlwaysRecordsReadByDecider(t *testing.T) { + setupTestDB(t) + ctx := context.Background() + + sid, err := CreateSession(ctx, nil, "pi", "权限", "") + if err != nil { + t.Fatal(err) + } + mailID, err := CreateMail(ctx, sid, nil, "pi", "", "jianf", "", + "权限请求: 请求执行 bash", "要跑 bash", nil) + if err != nil { + t.Fatal(err) + } + // 建一条待决的权限请求 + if _, err := db.DB.ExecContext(ctx, + `INSERT INTO permission_requests (request_id, mail_id, session_id, agent_name, question, options, context, created_at) + VALUES ($1,$2,$3,'pi','要跑 bash','[]','{}',NOW())`, + uuid.New(), mailID, sid); err != nil { + t.Fatal(err) + } + + if _, err := DecidePermission(ctx, mailID, "jianf", "同意"); err != nil { + t.Fatalf("正常决策不该失败: %v", err) + } + + // 判据:mail_reads 必须有记录 —— 它才是未读判据 + var n int + if err := db.DB.QueryRowContext(ctx, + `SELECT COUNT(*) FROM mail_reads WHERE mail_id = $1 AND reader_name = 'jianf'`, mailID).Scan(&n); err != nil { + t.Fatal(err) + } + if n == 0 { + t.Fatal("★ 决策后必须记 mail_reads:没有它这封信在收件箱里永远显示未读") + } + // 且派生读数(收件箱口径)必须变成 read + var state string + if err := db.DB.QueryRowContext(ctx, + `SELECT `+readStateFor("'jianf'")+` FROM mails m WHERE m.mail_id = $1`, mailID).Scan(&state); err != nil { + t.Fatal(err) + } + if state != "read" { + t.Fatalf("决策后收件箱口径应为 read,实际 %q", state) + } +} diff --git a/server/internal/repo/repo.go b/server/internal/repo/repo.go index bdedd73..f474200 100644 --- a/server/internal/repo/repo.go +++ b/server/internal/repo/repo.go @@ -1116,6 +1116,33 @@ func AttachPermissionDeadlines(mails []models.Mail) { // decider 是**做出决策的人**(用户名):权限邮件对他也应当变成已读 —— 但只对他一个人, // 而不是像原先那样把邮件行的全局 status 一改(抄送的其他 Agent 会因此看不到它)。 func DecidePermission(ctx context.Context, mailID uuid.UUID, decider, decision string) (*models.PermissionRequest, error) { + // ★ 2026-10-01:空 decider **直接报错**,不静默跳。 + // + // 函数头那句「权限邮件对他也应当变成已读」在 decider 为空时**静默失效**: + // 下面那两段都是 `if decider != ""` 才做,而**未读判据是 mail_reads** + // (见 unreadFor / readStateFor),不是行级那列。 + // ⇒ 结果是那封邮件被判为 read,可收件人在收件箱里**永远看到它未读**。 + // + // 线上实测 13 封这样的历史邮件(收件人均为 jianf,2026-09-08 / 09-13)。 + // + // 为什么当时不报而现在报:那 13 封的成因已被后来的 decider 兜底堵住 + // (permission.go 会沿会话树上溯找人类),HTTP 路径也传 user.Username。 + // 但 repo 层是**所有调用方的门**——MarkMailRead 早就为同一件事挡了 + // (「已读按读者记录,reader 是必填语义」),只挡它一个等于**漏网**。 + // 静默跳过正是当初 `if decider != ""` 的写法问题:它把「不知道谁读的」 + // 记成「不用记」。 + // + // 方向上这也是保守的:报错会让调用方看到「决策没落库」, + // 而静默放行会让收件人反复看到一封已经决策完的权限询问(真会发生的形状)。 + // + // ⚠ 注释里刻意不写那个列名的点号形式:mail_status_readers_test.go 的 + // readRe 是**纯文本**匹配,会把注释里提到它也算成「新增一处读取」。 + // 改代码时若那条判据突然变红,先看是不是这里写了列名。 + if strings.TrimSpace(decider) == "" { + return nil, fmt.Errorf( + "DecidePermission: decider 不能为空(未读判据是 mail_reads," + + "空读者会让这封权限邮件在收件人收件箱里永远显示未读)") + } var pr models.PermissionRequest var optsJSON []byte @@ -1143,14 +1170,11 @@ func DecidePermission(ctx context.Context, mailID uuid.UUID, decider, decision s `UPDATE mails SET permission_result = $1, status = 'read' WHERE mail_id = $2 AND status <> 'archived'`, decision, mailID) - if decider != "" { - // 记到决策人名下(按读者记,见 repo.markReadFor) - _, _ = db.DB.ExecContext(context.Background(), - `INSERT INTO mail_reads (mail_id, reader_name) - SELECT $1, $2 - WHERE NOT EXISTS (SELECT 1 FROM mail_reads WHERE mail_id = $1 AND reader_name = $2)`, - mailID, decider) - } + _, _ = db.DB.ExecContext(context.Background(), + `INSERT INTO mail_reads (mail_id, reader_name) + SELECT $1, $2 + WHERE NOT EXISTS (SELECT 1 FROM mail_reads WHERE mail_id = $1 AND reader_name = $2)`, + mailID, decider) return &pr, nil }