`markDelivered` 的注释写「标不上不该让投递失败……代价只是下次重启可能再投
一次」。opencode 独立复核指出这句话的前提站不住,我认这个判断。
「下次重启」把正确性押在「重启 ⇒ 内存全丢 ⇒ 从库里重来」上。但
`deliveredMails` 是 `BoundedSet(MAX_TRACKED_MAILS=2000)`,**运行期就主动淘汰**,
不需要重启。于是有一条更窄的重投路径:
投递 X ⇒ add(X) → POST /mail/read 失败(库里仍是 unread)
→ X 超过 2000 被淘汰(内存忘了,库没忘)
→ catchUp 按 unread 捞回 X,内存 has() 挡不住 ⇒ 重投
正是 2026-09-26 那个症状本身(重投回声),只是窗口窄得多。
两格分别锁住前提与后果,都不靠注释断言:
⑤ `deliveredMails 确实是有界的` —— 从 index.mjs 取上限常量名,回 bounded.js
取其值并断言有限。有限 ⇒ 运行期会淘汰 ⇒ 「靠重启才丢」不成立。
⑥ `淘汰只丢内存、不回写库` —— 覆盖 BoundedSet.add 的整个淘汰循环,
断言其中无 /mail/read|markDelivered|post|status|client;并对照
markDelivered 的失败分支只打日志、不重试不回滚。
将来若让淘汰也落库,这格会红,提醒改的是注释而不是加静音。
变异自测(三个变异各被对应格抓住,非自说自话):
改成无界 `new Set()` → ⑤ 红
淘汰路径里回写库 → ⑥ 红
markDelivered 失败后 setTimeout 重试 → ⑥ 红
判据自带的两个坑留在注释里(第一版「怎么变异都不判红」的原因):
必须锚在 BoundedSet 的 add 上,否则裸 /add\(value\)/ 会先命中文件前面
BoundedMap 的同形 add;淘汰循环要带尾巴({0,320}? 惰性量词会在第一个终点
就停),否则紧跟其后的落库代码永远落在窗口之外,结构上不可能判红。
全量 526 格通过。未修 —— 本提交只把已知窗口钉成可判红的判据,
不动 `markDelivered` 的行为(改行为是另一次决定,且应先决定淘汰时
要不要落库)。
Co-Authored-By: pi <pi@agentmail>
165 lines
8.7 KiB
JavaScript
165 lines
8.7 KiB
JavaScript
/**
|
||
* ★ 投递即标已读 —— 防「桥重启 → 重投 → 回声」。
|
||
*
|
||
* # 缺陷(用户报的,2026-09-26)
|
||
*
|
||
* 投递路径只把 mail_id 记进内存的 `deliveredMails`,**不动库里的 status**。
|
||
* 而 `catchUp` 按 `status=unread` 拉 ⇒ 桥一重启(每次部署都会),积压的
|
||
* "未读"被当成离线漏投**再投一遍**:
|
||
*
|
||
* · 5 个 mail_id 各进了**两条不同 pi 会话**(04:54 一条、08:01 一条)
|
||
* · 同一封信投两次 ⇒ 两个 worker 各回一封 ⇒ 对方收到两封 ⇒ 各回两封…
|
||
* · pi 收件箱 287 封 unread 中 **187 封已经回过信了**
|
||
* · 两条会话各烧到 463 / 268 封
|
||
*
|
||
* 用户原话:「我都不记得我下达这个任务,是你的桥自动重投存在 bug」
|
||
* 「就是你的错误的重投机制造成了回声」
|
||
*
|
||
* # 判据要钉住什么
|
||
*
|
||
* 不是"有没有 markDelivered 这个函数"(那太弱),而是:
|
||
* ① `deliveredMails.add` **只能**出现在 markDelivered 内部 —— 别处直接 add
|
||
* 就是一条绕过标已读的重投路径(这正是缺陷的形状)
|
||
* ② markDelivered 必须真的 POST /mail/read
|
||
* ③ 三个投递点(SSE / 补投 / 决策回执)都走它
|
||
* ④ 标已读失败时"再投一次"的说法只有在**内存集合一起丢**时成立
|
||
* (见下面「淘汰只丢内存不丢库」那条)
|
||
*/
|
||
import { test } from 'node:test';
|
||
import assert from 'node:assert/strict';
|
||
import { readFileSync } from 'node:fs';
|
||
import { fileURLToPath } from 'node:url';
|
||
import { dirname, join } from 'node:path';
|
||
|
||
const src = readFileSync(
|
||
join(dirname(fileURLToPath(import.meta.url)), '..', 'src', 'index.mjs'),
|
||
'utf8',
|
||
);
|
||
|
||
const boundedSrc = readFileSync(
|
||
join(dirname(fileURLToPath(import.meta.url)), '..', 'lib', 'bounded.js'),
|
||
'utf8',
|
||
);
|
||
|
||
test('★ deliveredMails.add 只允许出现在 markDelivered 内部', () => {
|
||
/*
|
||
这一条是整组判据的核心。
|
||
|
||
任何别处的 `deliveredMails.add(x)` 都意味着「我接管了这封,但库里还是
|
||
unread」⇒ 下次重启重投。缺陷就是这么长出来的:四处投递各自 add,
|
||
没有一处标已读。
|
||
|
||
允许的行号集合:markDelivered 函数体的那几行。
|
||
*/
|
||
const lines = src.split('\n');
|
||
const fnStart = lines.findIndex(l => /^function markDelivered\(/.test(l));
|
||
assert.ok(fnStart >= 0, 'markDelivered 必须存在');
|
||
|
||
// 函数体到下一个顶层 } 为止
|
||
let fnEnd = fnStart;
|
||
for (let i = fnStart; i < lines.length; i++) {
|
||
if (/^\}/.test(lines[i]) && i > fnStart) { fnEnd = i; break; }
|
||
}
|
||
|
||
const strays = [];
|
||
lines.forEach((line, i) => {
|
||
if (!/deliveredMails\.add\(/.test(line)) return;
|
||
if (i > fnStart && i < fnEnd) return; // 在 markDelivered 体内,合法
|
||
if (/^\s*(\/\/|\*|\/\*)/.test(line)) return; // 注释里提到它(说明文字)
|
||
strays.push(`${i + 1}: ${line.trim()}`);
|
||
});
|
||
assert.deepEqual(strays, [],
|
||
`这些地方绕过 markDelivered 直接 add ⇒ 库里的 status 不会被更新 ⇒ 重启重投:\n${strays.join('\n')}`);
|
||
});
|
||
|
||
test('markDelivered 同时写内存与库(两处口径必须一致)', () => {
|
||
const i = src.indexOf('function markDelivered(');
|
||
assert.ok(i > 0);
|
||
const body = src.slice(i, src.indexOf('\n}', i));
|
||
assert.match(body, /deliveredMails\.add\(/, '要写内存集合');
|
||
assert.match(body, /client\?\.post\('\/mail\/read'/, '要写库里的 status');
|
||
assert.match(body, /mail_ids: \[mailId\]/, '按 id 标(不传 mail_ids 会被服务端要求 workspace)');
|
||
});
|
||
|
||
test('★ 三个投递点都走 markDelivered(少一处就是一条重投路径)', () => {
|
||
// SSE new_mail 主路径
|
||
// 注意:`return;` 后面可能跟行尾注释("// B-3 第 1 步:去重"),
|
||
// 所以不能写 `return;\s*\n` —— 那样会被注释挡住而误报缺失。
|
||
assert.match(src, /if \(!id \|\| deliveredMails\.has\(id\)\) return;[^\n]*\n\s*markDelivered\(id\);/,
|
||
'SSE 主投递路径');
|
||
// 心跳补投(同样:`continue;` 后有行尾注释)
|
||
assert.match(src, /if \(deliveredMails\.has\(ev\.mail_id\)\) continue;[^\n]*\n\s*markDelivered\(ev\.mail_id\);/,
|
||
'补投路径');
|
||
// 决策回执
|
||
assert.match(src, /if \(decisionMailID\) markDelivered\(decisionMailID\);/,
|
||
'决策回执');
|
||
});
|
||
|
||
test('判据自检:反例(别处 add、不标已读)必须判红', () => {
|
||
// 这正是修复前的形状 —— 判据若放过它,就防不住回归
|
||
const before = 'if (data?.mail_id) deliveredMails.add(data.mail_id);';
|
||
const lines = [before];
|
||
const fnStart = lines.findIndex(l => /^function markDelivered\(/.test(l));
|
||
assert.equal(fnStart, -1, '反例里没有 markDelivered ⇒ 回退到最弱判据');
|
||
assert.ok(/deliveredMails\.add\(/.test(before), '反例确实会 add');
|
||
assert.ok(!/mail\/read/.test(before), '反例确实不标已读 ⇒ 会判红');
|
||
});
|
||
|
||
/**
|
||
* ★ markDelivered 注释里「失败不阻塞」的前提,**只在内存集合也一起丢时成立**。
|
||
*
|
||
* 注释原文(index.mjs:143 起):
|
||
* 「标不上不该让投递失败(正文已经在 worker 手里,代价只是下次重启可能再投一次,
|
||
* 与旧行为一致、不更差)」
|
||
*
|
||
* 「下次重启」这四个字把正确性押在"重启 ⇒ 内存全丢 ⇒ 从库里重来"上。
|
||
* 但 `deliveredMails` 是**有界**集合(`BoundedSet`,MAX_TRACKED_MAILS=2000),
|
||
* **运行期就会主动淘汰**。于是存在一条不需要重启的路径:
|
||
*
|
||
* 1. 投递 mail_X ⇒ `deliveredMails.add(X)`(内存记下)
|
||
* 2. POST /mail/read **失败** ⇒ 库里 status 仍是 unread
|
||
* 3. 之后 X 因超过 2000 被 BoundedSet **淘汰**(内存忘了,库没忘)
|
||
* 4. `catchUp` 按 status=unread 捞回 X ⇒ 内存 `has()` 挡不住(已淘汰)⇒ **重投**
|
||
*
|
||
* 结果就是 2026-09-26 那个症状本身(重投回声),只是窗口窄得多。
|
||
*
|
||
* 下面两条分别锁住:淘汰是真实的(不靠注释断言),以及淘汰只丢内存不丢库。
|
||
*/
|
||
test('deliveredMails 确实是有界的(运行期会主动淘汰)', () => {
|
||
const i = src.indexOf('const deliveredMails = new BoundedSet(');
|
||
assert.ok(i > 0, 'deliveredMails 必须是 BoundedSet —— 无界就谈不上淘汰');
|
||
const m = src.slice(i).match(/new BoundedSet\((\w+)\)/);
|
||
assert.ok(m, '取得到上限常量名');
|
||
|
||
const cap = boundedSrc.match(new RegExp(`export const ${m[1]} = (\\d+)`));
|
||
assert.ok(cap, `bounded.js 里导出了 ${m[1]}`);
|
||
// 有限 ⇒ 运行期会淘汰 ⇒ 「下次重启才丢」这个前提站不住
|
||
assert.ok(Number(cap[1]) < Infinity, `${m[1]} 是有限值(当前 ${cap[1]})⇒ 不是靠重启才丢`);
|
||
});
|
||
|
||
test('淘汰只丢内存、不回写库 ⇒ 标已读失败 + 淘汰 会重投(已知窗口)', () => {
|
||
// BoundedSet 淘汰路径里没有 /mail/read —— 也就是说淘汰**不会**把
|
||
// 被淘汰的那封补标成已读。这条断言的作用是把上面第 2、3 步钉死:
|
||
// 若将来有人让淘汰也落库,这个窗口就自动关上了,届时这条应当判红提醒改注释。
|
||
// ★ 两个坑叠在一起才导致第一版「怎么变异都不判红」,留注释免得再犯:
|
||
// ① 必须锚在 BoundedSet 的 add 上:裸 /add\(value\)/ 会先命中文件前面
|
||
// BoundedMap 的同形 add,非贪婪匹配选错了类;
|
||
// ② 必须覆盖**整个淘汰循环**、且用捕获组取到结尾:`/this\.evicted\+\+/`
|
||
// 本身就匹配全了(没有 [^]* 尾巴),{0,320}? 惰性量词在第一个终点就停,
|
||
// 于是紧跟其后的落库代码**永远落在窗口之外**,断言结构上不可能判红。
|
||
const setClass = boundedSrc.slice(boundedSrc.indexOf('export class BoundedSet'));
|
||
assert.ok(setClass.length > 0, '找得到 BoundedSet 类');
|
||
// 抓整个 while 淘汰循环:从 while 到 return this
|
||
const evictLoop = setClass.match(/while \(this\.set\.size > this\.limit\) \{[\s\S]*?\n {4}\}/);
|
||
assert.ok(evictLoop, 'BoundedSet.add 里有淘汰循环(说明淘汰真会发生)');
|
||
assert.ok(!/\/mail\/read|markDelivered|post\(|status|client/.test(evictLoop[0]),
|
||
'淘汰循环只丢内存、不写库 ⇒ 「post 失败 + 淘汰」= 重投。本测试锁住这个窗口;'
|
||
+ '若将来让淘汰也落库,这条会判红,届时应改的是 markDelivered 的注释。');
|
||
|
||
// 对照:markDelivered 里的失败分支只打日志,不重试也不回滚。
|
||
const fn = src.slice(src.indexOf('function markDelivered('), src.indexOf('\n}', src.indexOf('function markDelivered(')));
|
||
assert.match(fn, /\.catch\(/, 'post 失败被吞掉(这是「不更差」的前提来源)');
|
||
assert.ok(!/retr(y|ies)|setTimeout|unmark/.test(fn),
|
||
'失败后不重试、不回滚 ⇒ 依赖「重启时内存全丢」兜底,而淘汰会提前破坏这个前提');
|
||
});
|