fix(权限): 决策人��空时不再静默跳过已读记账 —— 那让权限邮件永远显示未读
## 形状
`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 是必填语义),只挡它一个等于漏网。
静默跳过正是 `if decider != ""` 的写法问题:它把「不知道谁读的」记成「不用记」。
## 同族核对(不是只看这一处)
全 internal/ 树扫 `UPDATE mails SET status ... 'read'`,共 4 处,
逐处核对其前置 45 行是否有 mail_reads 记账:
MarkMailRead ✓ / MarkMailsReadFor ✓ / MarkAllInboxReadForSession ✓ / DecidePermission ✗
⇒ 唯一缺口就是这一处。事实表本身干净:无空读者、无孤儿行。
## 判据
新增 2 格:空 decider 报错 / 决策后 mail_reads 必有记录且派生读数为 read。
两个变异都经得起(去掉报错守卫、去掉 mail_reads 写入,各红一格)。
## 顺带记一个判据的坑
`mail_status_readers_test.go` 的 readRe 是**纯文本**匹配 `mails.status`,
会把我注释里提到该列名也算成「新增一处读取」,导致那条清册判据变红。
已在注释里避开那个写法并写明原因 —— 改这块代码时若它突然变红,先看是不是注释。
This commit is contained in:
@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@ -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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user