feat(server): review 截图面行账保留策略(INT-184) #166
No reviewers
Labels
No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
fjbobo/fast_ship!166
Loading…
Reference in a new issue
No description provided.
Delete branch "int-184/asset-retention"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
相关需求
INT-184(审查截图面持久资产缺少保留策略):http://192.168.31.31:4888/projects/5ca4b87d-7014-4d48-809e-b0c80cd9fff7/issues/80effd31-a4c0-4bdf-afb6-8d605fd49686
改动内容
review-screenshot-cache面此前只增不减(跨 head 累积、探测落盘无行引用、崩溃残留都无人回收)。本 PR 按 INT-184 的方向审计结论以 review_assets 行为唯一账本重建回收,不复活目录推断:reviewAssetRetention):created_at超龄的资产行退役。关闭 PR、旧 head、「探测过但没跑视觉」的字节按同一条年龄规则退出,不为触发条件引入 PR 关闭态这样的第二状态源。probeAndRecordScreenshots(decideVisualForPR调用点),成功下载的引用当场 upsert 资产行——探测字节从落盘起就有行引用,R12 反例 2(探测与视觉运行间隔超宽限被误删)按构造消除。纯决策入口probeScreenshots保持原语义;失败行仍由ensureAssets落账并保留重试语义;已有成功行不被探测改写。assetFileExists(storage_path),文件缺失按失败重下并更新原行——任何删除竞态残留都自愈,不再「从此 404」。fetchAndCache的 FromCache 返回统一os.Chtimes刷新.binmtime(touchReviewEntry,媒体面 no-op)——行插入路径的 mtime 必新,R12 反例 3 的「命中旧条目不刷新 mtime」封口。assetRetentionLoop(6h + 启动一轮,与 redeliveryLoop 同款独立循环)先删超龄行,再把存活storage_path集合交给 githubmedia 新导出的SweepUnreferencedEntries(面内唯一 sanctioned 删除路径)逐.bin处理:先查引用、后查 mtime(宽限 24h)、再删,.bin与同名.json一起删,孤儿 meta 同规则回收;各步幂等,任意时点被杀下一轮收敛。docs/internal/github-media-proxy.md、docs/internal/reviews.md同步更新。审查加固(18 轮云端审查累计处置,详见各轮处置评论)
R6–R16 在保留策略上连续叠加了十层防线,全部经方向审计(有限清单定性维持)与再审计(证伪条件判不翻转)确认收敛:
id IN + COALESCE(renewed_at, created_at) < + NOT EXISTS 在途运行单条语句,消除选取与删除之间的窗口(重开/重跑的在途运行、补账续的新时钟都被复检保住)。os.Chtimes在 DELETE 之前执行——宽限(24h)从退役时刻起算,退役后新建运行在宽限内仍能看到文件。renewed_at:created_at恢复不可变语义(首次落账时刻,供AgentScreenshots判「本轮新行」);补账续期只刷renewed_at——两个时钟不得混用。completed的视觉运行(在途/失败/取消不作基准);截止取min(上一轮 finished_at, 本轮 created_at)——重叠轮次(manual rerun 恒开新轮)下新增截图不再被误并。storage_path不因一次失败重下清空(ENOENT 也可能来自卷瞬时不可用)——卷恢复后的文件不再成孤儿。idx_review_assets_storage_path+idx_review_assets_retire_clock(表达式索引)让终检/退役定位从全表扫描降为索引查找。model.ReviewRunActiveStatuses切片生成(全集对账单测钉住),新增状态不再静默漂移。修改范围
server/internal/pkg/githubmedia/proxy.go(面不变量注释、SweepUnreferencedEntries、touchReviewEntry)server/internal/service/review_asset_retention.go(新增:保留策略循环)、review_assets.go(探测落账 + 文件复核)、review_sync.go(调用点)、review_scheduler.go(Start 挂循环)、cmd/server/main.go(装配注释)docs/internal/github-media-proxy.md、docs/internal/reviews.mdweb/vite.config.ts(无关基线测试修复,见下)本地验证
cd web && pnpm api-types --checkmake lintmake testgo test ./...全绿;web vitest 146 文件 / 1768 测试全过go test ./...(含新增保留策略/探测落账/文件复核测试,真实文件系统 + 真实 githubmedia 服务);无头 Playwright 驱动隔离实例注册 → 仪表盘渲染(1440×900 截图 + ARIA 快照)verify-fast-ship技能隔离启动(独立端口 5088 + 独立 sqlite/uploads,等价make dev但不占共享端口;doctor 检查全过:进程/端口/静态托管 bundle 一致/API 探针/启动日志)go test -count=1 ./...全量绿)本 PR 无界面变化,端到端证据为隔离实例上的真实二进制行为演示:种入 4 行 + 5 个文件(含「一老一少两行共享同一 storage_path」、48h 前的孤儿、新鲜孤儿)后重启服务,启动清扫日志
截图资产保留策略扫描 retired_rows=2 removed_files=2;扫描后超龄行删除、aged.bin/orphan_old.bin回收、shared.bin因年轻行仍引用而保留、宽限内的新鲜孤儿保留——与验收口径逐条对应。另有go test覆盖:共享 storage_path 双 head 不删、探测字节退役回收、连跑三轮收敛(可重入)、缺失文件自愈重下、探测落账三态(新建/修复失败行/不改写成功行)。已修复的基线问题
markdown-editor与issue-pages两个 vitest 套件整体起不来(@uiw/react-md-editor的 ESM import CSS 外部化后落到 node 原生 loader,Unknown file extension ".css"),1721 过 / 47 个测试被吞。发现命令:make test(web vitest)。修复提交:34104b2f(test.server.deps.inline内联进 vite 转换管线),修复后 146 文件 / 1768 测试全过。端到端截图
无界面变化(纯 server + 一处 web 测试配置基线修复),按模版说明删去对比表,替代证据见「本地验证」末段:启动清扫日志行、扫描前后的文件/行状态、无头仪表盘截图(
01-dashboard-after-boot.png,仅证明应用可启动,不含任何本 PR 引起的界面差异)。本地审查
创建 PR 前已完成一轮双模型独立审查(grok-4.7 + glm-5.3-flash,只读):grok-4.7 指出 1 个 P1(引用快照与删除之间无互斥,
DirEntry.Info可能是 ReadDir 时快照,mtime 兜不住窗口)与 3 个 P2(探测落账 DoNothing 吞行、assetFileExists把所有 Stat 错误当缺失、五处「ensure 不会重下」注释漂移),已在d2916433逐条修复并补测试(持下载闸复核、崩溃残留兜底回收、非 ENOENT 保持原行等)。glm-5.3-flash 的结论合并后如有新增必改项同样修完再推送。未验证范围与原因
created_at/mtime 模拟超龄,无法等待真实 30 天;行为由单测与隔离实例演示覆盖。API 变更说明
无。
创建 PR 前完成
审查意见处理
$pr-review-loop自动等待审查结果并处理新意见,直到收敛(第 18 轮 fastship-bot 0×P0–P2 + 0 Note 全部通过,CI 检查全过,已收敛)@codex review
INT-184:review 截图面行账保留策略(行保留 30 天 + 探测落账 + ensure 复核自愈 + 命中 touch + 行账回收持下载闸)。本地已过双模型审查(grok-4.7 / glm-5.3-flash),P1/P2 已修(
d2916433)。请重点看SweepUnreferencedEntries的持闸删除与孤儿判据兜底、探测落账的并发语义。模版见 PR 描述,基线 4412cd13,HEAD6ff2f578。代码审查 ·
38262fb· 第 18 轮全部组合通过,未发现 P0–P2 问题。
cliproxyapi/deepseek-v4.1-flash · pi
thinking=low
复审说明:推送新提交后会自动重新审查。
本轮未发现 P0/P1:行账回收的整体安全论证成立,剩余问题集中在「复用旧行不续期」与几处删除/判定边界。影响是 head 超龄后被重新审查时可在运行中失去行与文件、以及极端 Stat 失败下 meta 与 .bin 配对被破坏。
@ -1009,0 +1088,4 @@}metaPath := filepath.Join(s.cacheDir, name)binPath := strings.TrimSuffix(metaPath, ".json") + ".bin"if _, err := os.Stat(binPath); err == nil {P2:问题:孤儿 meta 遍历把
os.Stat(binPath)的任何错误都当成「data 不在」,EIO/EACCES 与 ENOENT 一视同仁;key 为空的分支在 stale() 判定与 os.Remove 之间无锁,可删掉并发重写出来的新 meta。影响:仍被 review_assets 行引用、.bin 实际存在的条目会丢 meta(mime/scope/size 与闸 key 的事实源),之后只能走 servePersistedData 魔数兜底,与同 PR 的 assetFileMissing「只有 ENOENT 才算缺失」口径不一致。建议:用 errors.Is(err, os.ErrNotExist) 判定缺失,并在持闸后按 fetch 侧口径复核 meta 未被替换再删。@ -0,0 +64,4 @@// storage_path 集合回收无引用字节。行退役失败或引用集合读失败时直接返回,// 不动文件——账不确定时宁可保守(下轮重试)。func (s *ReviewService) sweepAssetRetention(now time.Time) {res := s.db.Where("created_at < ?", now.Add(-reviewAssetRetention)).P2:问题:行退役只看 created_at,而 ensureAssets 命中已存在行、recordProbeAssets 修复旧失败行时都不刷新 created_at,也不 touch 文件 mtime(AgentScreenshotImage 直接 os.Open,不经缓存面)。影响:head 超过 30 天后被重新审查(重开/手动重跑/评论唤醒)时,本次运行生成的 asset id 与文件都可能在随后 6h 扫描中被删——行按首次下载时间退役,文件 mtime 同样超龄、24h 宽限兜不住,取图 404 或截图列表变空;ensureAssets 的复核发生在删除之前,救不回来。建议:复用行时刷新 created_at(或按最后引用计算保留期),或在有在途/非终态视觉运行的 head 上跳过退役。
R2 意见处理(
1dfc63df)四条全部 Act on,已修复并推送:
changed在截图未变的重跑上误报 true;跳过退役不影响指纹口径、不动 schema。新增 TestAssetRetentionSkipsHeadsWithActiveRuns(running 保护 / 终态后恢复退役)。errors.Is(err, os.ErrNotExist)才算 data 不在(与 assetFileMissing 同口径);key 已知分支持闸后重读 meta 复核闸 key 未变(未被并发重写)再删;key 空分支删除前重读,已被重写成可解析条目则不删。os.Remove(binPath)成功或本就 ENOENT 时才删 meta;EIO/EACCES 失败保留 meta(URL/Scope/闸 key 事实源),下轮重试。server 全量
go test ./...+ gofmt + vet 通过。已按「等待上限只花一次」记录:codex 本轮超时未返回(请求后 30 分钟无结论),本轮不再等待;后续轮次若它自行给出结论照常回收。推送自动触发新一轮审查。本轮未发现新的功能性缺陷,行账退役、探测落账与文件回收的主要路径自洽;主要隐患是退役 SQL 的关联子查询缺少可用索引,首次扫描会长时间占用 SQLite 单写锁。
@ -0,0 +71,4 @@// 接读 storage_path,不经缓存面)——无守卫的话,在途运行会在两次扫描之// 间失去行与文件(取图 404、截图列表变空),ensureAssets 的复核发生在// 删除之前救不回来。运行进终态后,下一轮恢复按年龄退役。res := s.db.Where("created_at < ?", now.Add(-reviewAssetRetention)).P2:问题:退役 DELETE 的 NOT EXISTS 子查询按 (integration_id, repo_full_name, pr_number, head_sha, status) 关联 review_runs,但现有索引都以 project_id 开头(idx_review_runs_pr_head / idx_review_runs_combination),子查询无法按前导列直接用索引(无统计信息时退化为逐行全扫 review_runs,ANALYZE 后也只能 skip-scan project_id);review_runs 没有任何清理路径,随时间单调增长。影响:每 6h 一次(含进程启动即扫)的 DELETE 会长时间持有 SQLite 唯一写锁,首次部署/长时间停机后待退役行最多,30s busy_timeout 内的调度写、webhook 写会排队甚至超时。实测(sqlite3 复刻真实索引)5 万待退役资产 × 20 万 run:无统计信息时 >600s,ANALYZE 后仍 126s;加 (integration_id, repo_full_name, pr_number, head_sha, status) 索引后同数据约 2s。建议:为守卫查询加匹配索引或先一次性查出在途 head 集合再在 Go 侧排除,并按批(如每轮 LIMIT 一批)退役;同时给 review_assets.created_at 建索引避免每轮全表扫描。
R3 意见处理(
8a4bc933)P2(退役 SQL 索引与写锁占用)Act on,两条建议都采纳:
idx_review_runs_asset_head(integration_id, repo_full_name, pr_number, head_sha——守卫探测的关联列)与idx_review_assets_created_at(超龄定位)。status 不入键:按 head 命中的行数极小,状态过滤在行上做等价。DELETE FROM review_assets WHERE id IN (SELECT a.id ... LIMIT n)(走上述两个索引),每批独立语句、批间释放写锁;reviewAssetRetireBatch = 2000改 var 供测试注入。批失败即停:账不确定时保守返回,已删批次的字节由下轮扫描收敛(路径幂等可重入)。新增
TestAssetRetentionRetireBatches:批量上限 2、待退役 5 行 → 多批删到收敛,退役后同轮扫描回收文件(文件 mtime 回拨模拟生产时序)。server 全量go test -count=1 ./...+ gofmt + vet 通过。推送后自动触发下一轮审查。另:R1 行内评论(#16646)在本轮通知中重放了一次,其内容已在新 HEAD 修复,未另计轮次。
本轮未发现 P0/P1 级新问题,保留策略的引用计数、持闸复核与分批退役路径与测试一致;余下为索引建失败无痕迹与注释口径漂移两类非阻塞项。影响限于索引创建失败的极端启动场景会静默退回慢查询,以及后续维护者可能被「永不回收」的旧注释误导。
@ -856,0 +860,4 @@// 前导列可用的关联查询在全量扫描下会让每 6h 一次的 DELETE 长时间占住// SQLite 单写锁(R2 审查实测:5 万待退役×20 万 run 时 >600s,加索引后// 约 2s);service 层另按小批量执行,批间释放写锁。db.Exec("CREATE INDEX IF NOT EXISTS idx_review_assets_created_at ON review_assets(created_at)")P2:问题:两条新索引用裸 db.Exec 建出且忽略 .Error,而同函数里的 host_key 索引是有检查并会启动失败的;影响:启动时若拿不到 SQLite 写锁或磁盘满导致建索引失败,分批退役每批仍会对 review_runs 逐行全扫(上轮实测 5 万×20 万时单批即分钟级),把刚修的「长持写锁拖垮调度/webhook 写」静默带回来且日志无痕;建议:检查这两条 CREATE INDEX 的 .Error,至少 warn 记录「缺索引、退役将慢」,或与 host_key 索引同款失败即启动失败。
R4 意见处理(
97e74442)连续三轮 Act on 已按规则触发方向审计(claude-opus-5-5,独立顾问),定性有限清单:方向成立、无共同根因(新代码缺陷递减尾部 4→1→1),沿当前方向修完即收敛。两项处置:
CREATE INDEX失败(磁盘满/锁/损坏)→return fmt.Errorf启动中止;表不存在(部分 schema 调 createBootIndexes 的测试路径)时跳过,AutoMigrate 建表后下次启动自然补上。review_assets.go两处保留「磁盘必须在源头设闸」的论证(改为「回收滞后至少 30 天,源头闸仍然必要」);顺手修掉一处注释断行。另采纳审计建议:退役 SQL 的非终态状态列表从 model 常量拼接,新增状态时不再在 SQL 字面量里静默漂移。
server 全量
go test -count=1 ./...+ gofmt + vet 通过。审计定性已入跨轮账本。推送自动触发下一轮审查。本轮未发现 P0/P1:R4 的索引失败启动报错与注释口径修复均已核实,保留策略主体逻辑与测试一致;只剩退役/回收之间的边界竞态与两处口径缝隙。影响限于超龄 head 重审瞬间及探测异常字节,不改变主线正确性。
@ -0,0 +106,4 @@LIMIT ?)`, strings.Join(activeStatusLiterals, ", "))for {res := s.db.Exec(retireSQL, now.Add(-reviewAssetRetention), reviewAssetRetireBatch)P2:问题:退役守卫(NOT EXISTS 非终态运行)只在 DELETE 那一刻判定,之后同轮立刻按快照回收文件,两步之间没有原子性。影响:若手工重跑/评论唤醒/pr-review 在 DELETE 之后、引用集合快照之前为该超龄 head 建了 queued 运行,该运行的资产行已被删、文件 mtime 又是 30 天前的旧值(ensureAssets 命中不重下、不走 touch),同轮 SweepUnreferencedEntries 会把文件真删掉;随后运行侧 AgentScreenshots→ensureAssets 只能重下,上游附件已删/私有附件令牌失效时该轮截图永久缺失,且该 head 的历史行也已不存在。建议:把本轮退役的 storage_path 留到下一轮再扫(宽限从退役时刻起算),或文件回收前按 head 重新确认无在途运行后再删。
R5 意见处理(
b0d952a5)P2 + 3 条 Note 全部 Act on:
server 全量
go test -count=1 ./...+ gofmt + vet 通过。推送自动触发下一轮审查。本轮保留策略实现整体自洽,新发现 1 个 P2(补账/复用资产行不续期,刚下载的字节会被提前回收)与 2 条回收路径边界隐患,无 P0/P1。界面无改动(仅服务端与 vitest 配置)。
@ -616,0 +671,4 @@}if uerr := s.db.Model(&model.ReviewAsset{}).Where("id = ?", existing.ID).Updates(map[string]any{"storage_path": f.cached.Path, "mime_type": f.cached.Metadata.ContentType,P2:问题:探测补账(recordProbeAssets 修复空 storage_path 行)与 ensureAssets 补账(review_assets.go:493 命中已有文件直接跳过)都不刷新 created_at,命中路径也不再 touch mtime。影响:一个 30 天前首次记录为失败、或首次审查就在 30 天前的行,即使今天刚重新下载并供本轮视觉运行使用,仍处于超龄状态——运行结束后退役守卫失效,下一次扫描就删行,文件因刚写过只再活一个 24h 宽限窗(约 24–30h)即被回收;重审超龄 head 或修复失效图链后,刚用过/刚下好的截图证据在一天内消失,上游已删或转私时不可恢复。建议:把空 storage_path 被填上的补账视作一次新的资产落账(刷新 created_at),或增加 fetched_at 并让保留期从最近一次成功写入起算。
R6 意见处理(
abf62850)P2 + 2 条 Note 全部 Act on:
created_at = now(补账即落账,保留期时钟从最近一次 materialization 起算);失败重试不刷新,避免僵尸失败行永不过期;ensureAssets 的 plain hit(行与文件都在)不刷新——那里字节与账同龄、超龄退役是诚实语义,且刷新会破坏 AgentScreenshots 的 prevRefs 时间过滤(同 head 重跑 changed 误报)。fetched_at 独立列方案评估过:需要迁移 + 存量回填,本 PR 不引入;若后续需要区分「首次记录」与「最近 materialization」再立项。os.Chtimes续期到本轮扫描时刻,「宽限从退役时刻起算」真正成立;退役后新建的排队运行只要在 24h 宽限内启动,ensureAssets 缓存命中 touch 续保。SweepUnreferencedEntries 增加 now 参数(扫描时刻由调用方传入,测试可确定性推进时间)。测试:补账续期断言 ×2(重下成功/修复失败行)、退役即续期后宽限外回收、+25h 扫描越宽限的时间轴适配。server 全量
go test -count=1 ./...+ gofmt + vet 通过。推送自动触发下一轮审查。R6 三项修复均已落地并有测试覆盖,本轮未发现会必然触发的故障。剩余风险集中在唯一删除路径的防线:引用快照与缓存目录失配时按「全无引用」整批删除,且 DB 复核错误、退役续期失败都是静默失效,可能永久丢失上游已删的历史截图。
@ -0,0 +186,4 @@return true}return false}, reviewAssetSweepGrace, now)P2:问题:传给 SweepUnreferencedEntries 的引用集合只来自 review_assets 表,没有任何「快照与当前缓存目录同源」的自检(快照为空、或路径前缀整体不匹配都直接当作「目录里全是孤儿」)。影响:上传存储路径迁移、用旧库备份恢复、新实例挂载旧缓存卷这类运维动作下,目录里的 .bin 全部无行引用,24h 宽限后整批删除;这是本 PR 首次引入的删除路径,此前这些字节永远不会被删,上游已删/转私的截图因此永久丢失。建议:单轮扫描前后记录快照规模与目录条目规模,遇到「快照为空而目录有条目」「目录条目数远大于快照条数」只告警并分档放量(或要求显式开关),不要一次删干净。
R7 意见处理(
3a10aa4c)P2 + 2 条 Note 全部 Act on:
reviewAssetSweepCalmBatch(25)个文件并 warn 留痕(reason/dir_candidates/referenced/budget 全量日志)。误删从一次清空降为缓慢滴漏,运维看到 warn 可介入;正常排水的尾巴也走限量,收敛变慢但不永久遗留。机制上新增 reviewAssetSweeper 的 CacheDir/CountCacheEntries(ProxyService 直读 cacheDir,无新迁移)。测试:限量模式各档、fail-closed、+25h 扫描越宽限的时间轴适配。server 全量
go test -count=1 ./...+ gofmt + vet 通过。推送自动触发下一轮审查。R7(第 7 轮)Note 处置(
60a7e302)第 7 轮 0×P0–P2,3 条 Note 全部 Act on:
paths(与引用快照同名遮蔽)确认是续期改序时的重构残留,移除。WithSweepLogger选项(生产装配appLogger),SweepUnreferencedEntries与CountCacheEntries两处os.ReadDir失败都限频 warn(含 cache_dir 与 error,间隔同 meta 读失败口径);枚举失败时本轮不删任何文件(保守暂停),管理员有痕可查。选 logger 注入而非接口返回错误,是让告警留在唯一有部署日志管道的装配点,不扩散 sweeper 签名。reviewAssetSweepCalmBatch(25)个、引用文件保留、下轮继续限量滴漏。fail-closed 的注入(让 live 复核查询报错)在现有 sqlite 测试里做不了干净注入(Pluck 先行失败会提前返回),该分支维持 review 审读;三档限量分支已有行为级覆盖。server 全量
go test -count=1 ./...+ gofmt + vet 通过。推送自动触发下一轮审查。R8 未发现 P0/P1;新增的枚举失败告警与残留切片清理已落地,但回收删除的「最终 DB 复核」被 allowed 短路绕过,留着一条窄窗口的数据丢失路径。另有两条 Note:两档限量测试的断言恒真、限量预算在判定前扣减导致空转。
@ -0,0 +234,4 @@if _, ok := referenced[binPath]; ok {return true}if allowed[binPath] {P2:问题:isReferenced 闭包对同一候选的第二次调用被
allowed[binPath]直接短路返回 false,不再查库;而 SweepUnreferencedEntries 的删除路径正好在持闸后第二次调用 isReferenced(经 unreferencedAndStale)。第一次调用发生在取闸之前,行插入发生在写侧释放闸之后,两者之间正是需要复核的窗口。影响:注释承诺的「候选删除前做一次真实 DB 复核作最终闸」在最需要它的场景(touch 失败,如备份恢复后文件属主不同导致 os.Chtimes 静默失败、mtime 停在旧值)不生效——快照之后插入的 review_assets 行仍可能指向被删掉的持久截图,上游已删/转私时不可恢复。建议:第二次调用一律重新查库(可保留 allowed 只为预算不重复扣减),或把「允许删除」的判定整体挪到持闸后的最终复核里。R8 意见处理(
1193d1ab)+ 方向再审计结论第 8 轮 1×P2 + 2×Note 全部 Act on。因「回收防线」子系统已连续 3 轮有 Act on(R6/R7/R8),按流程先做了方向再审计(Claude Opus 5.5,独立只读审整份 diff),定性:有限清单(维持)——共同根因是 isReferenced 闭包同时承担「时敏安全谓词」与「预算记账」两个契约,R7 把它们焊进了同一个函数;R6 之外各层零新增意见,修完本轮后该类问题整体消失。审计同时给出证伪条件:若下轮 Act on 落在本轮新代码且引入新故障模式类,定性翻转为开放反例、换候选集 pipeline 路径。
修复(按审计确认的顺序落地):
server 全量
go test -count=1 ./...+ gofmt + vet 通过。推送自动触发下一轮审查。本轮未发现 P0/P1 级问题;行账回收的引用判定、宽限、预算与退役顺序在最新一轮修复后自洽,残留 1 个 P2 查询放大与 1 个 Note 预算口径问题。影响面限于大存量/异常挂载下的扫描耗时与收敛速度,不改变删除正确性。
@ -0,0 +247,4 @@// 做一次真实 DB 复核作最终闸(R6 审查意见),每次询问都真查库// (R8 审查意见)。候选稀少,无索引全表扫描的代价可忽略。var live int64if err := s.db.Model(&model.ReviewAsset{}).Where("storage_path = ?", binPath).P2:问题:候选删除前的 DB 终检用
WHERE storage_path = ?,review_assets 上没有 storage_path 索引,而 R8 修复让同一候选在入口与持闸终检各查一次真库。影响:退役/孤儿堆积(本 PR 正是为清理存量而做,首次上线可能有上万候选)时,单轮扫描等于上万次对 30 天面全表扫描,扫描时长与持闸时间一起放大(闸内查询还会阻塞同 URL 下载最多数秒),6h 周期的回收可能长时间跑不完甚至跨轮重叠。建议:给 review_assets(storage_path) 加索引(createBootIndexes 里与 created_at 索引同批建),或把终检改成「快照后新增行」的一次性查询(created_at > 快照时刻 OR id 不在快照里),把每候选两次全表扫描降为常数。R9 意见处理(
ab523730)第 9 轮 1×P2 + 1×Note 全部 Act on。两条意见按再审计的证伪条件判定(均不改变删除正确性、属既有查询/记账模式的放大而非新故障模式类)→ 维持有限清单,不触发翻转。
createBootIndexes新增idx_review_assets_storage_path(与 created_at 索引同批:CREATE INDEX IF NOT EXISTS+ 表存在检查 + 失败即启动错误)。已用 EXPLAIN QUERY PLAN 验证终检查询命中 covering index search(全表扫描 → O(log n) 查找),闸内查询耗时降为微秒级,不再阻塞同 URL 下载。未选「快照后新增行一次性查询」方案:终检的语义是按候选即时判定(快照之后的行持续插入),集合化需要引入快照时刻后的行缓存与失效逻辑,正是再审计预警过的第 N+1 层防御;索引让每候选查询保持精确且新鲜。delete(charged)+budget++,限量口径收紧为「每轮最多 25 个删除」。边界明确化:闸占超时/删除失败仍不退(sweeper 侧无回传通道;预算每轮重置,下一轮自愈)——把结算搬到删除成功点需要 sweeper 回传每个候选结局,即再审计预留的候选集 pipeline 翻转路径,本轮按预留条件不启用。server 全量
go test -count=1 ./...+ gofmt + vet 通过。推送自动触发下一轮审查。本轮只发现 1 个 P2 竞态与 1 个注释错挂:退役 DELETE 不复检在途运行守卫,重开/重跑的旧 head 可能在选取与删除之间被删行。影响限于在途视觉运行的截图列表/取图 404,下一轮 ensureAssets 会自愈,不会误删文件或数据。
@ -0,0 +170,4 @@}}}if err := s.db.Where("id IN ?", ids).Delete(&model.ReviewAsset{}).Error; err != nil {P2:问题:每批先 SELECT 带 NOT EXISTS(非终态运行) 与 created_at < 截止,再
Where("id IN ?", ids).Delete(...),DELETE 只按 id 删,既不复检在途运行也不再校验 created_at。影响:SELECT 与 DELETE 之间(本段还逐行 os.Chtimes,网络盘/慢盘上是秒级窗口)若该 head 被评论唤醒/手动重开跑出 queued/running 运行,或某行被 ensureAssets 重下、recordProbeAssets 修复时用新 created_at 续期,这些行仍会被删;行消失后文件只靠 24h mtime 宽限存活,在途运行会拿到空截图列表,或先拿到列表再按旧 assetId 取图时 404(ensureAssets 重建行会换 id)。建议:DELETE 复刻选取条件(id IN ? AND created_at < ? AND NOT EXISTS(在途运行)),或把选取与删除放进同一事务/一条带子查询的 DELETE,让守卫与删除原子化。R10 意见处理(
707faad4)第 10 轮 1×P2 + 1×Note 全部 Act on:
DELETE ... WHERE id IN (?) AND created_at < ? AND NOT EXISTS(在途运行),与 SELECT 共用同一组状态字面量(同一activeStatusLiterals)与同一个截止时刻变量,守卫复检在单条语句内原子化:选取与删除之间重开/重跑冒出的 queued/running、补账续的新 created_at 都不再被误删。附带两处加固:retired计数改按RowsAffected(复检拦截的行不再计入账目);整批守卫翻案(deleted=0 且满批)时停批,防止同批重选死循环。选删之间的 Chtimes 续期保持原序——多 touch 无害,被复检保住的行宽限从本轮起算依然正确。touchReviewEntry的不变量说明(含「行插入路径的 mtime 必新」)紧贴其函数,logSweepDirReadFail带自己的单行说明。未加「选删之间翻转」的直接单测:翻转发生在 SELECT 与 DELETE 两条语句之间,现有测试桩无法在不引入注入钩子的情况下插入(加钩子本身是再审计预警过的防御层繁殖);两侧条件由同一字面量组与同一变量构成,一致性由编译期单一事实源保证。
server 全量
go test -count=1 ./...+ gofmt + vet 通过。推送自动触发下一轮审查。本轮未发现新的 P0/P1;R10 的 1×P2 + 1×Note 已核实修复。唯一新问题是 review_assets.created_at 复用为保留期时钟后,与 AgentScreenshots 的「上一轮行」过滤语义冲突,同 head 重跑且文件缺失重下时会误报截图发生变化。
@ -506,0 +517,4 @@"size_bytes": size, "error": errText,}if storagePath != "" {updates["created_at"] = nowP2:问题:本 PR 把 review_assets.created_at 复用为「最近一次 materialization/保留期时钟」,成功重下(此处)与探测修复失败行(recordProbeAssets 同文件 679-684 行)都会把它刷到当前时刻;但 AgentScreenshots 第 920 行仍用 a.CreatedAt.After(run.CreatedAt) 当作「该行是本轮才写入的」来剔除、构造上一轮截图集合 prevRefs。影响:同 head 的手动重跑/重开时(注释已说明该路径会再跑视觉轮),若上一轮某行文件缺失触发重下(文件被外部清理、部分磁盘恢复、并发删除竞态),该行 created_at 被刷到本轮 run 之后而被当成新行剔除,prevRefs 变短或为空,changed 误判为 true,视觉审查会把未变化的截图集合报成「截图已变化」,同一漂移也会出现在探测修复失败行后的集合判定上。建议:把「行首次出现时刻」与「保留期时钟」拆开(新增 first_seen_at 列,或只让保留策略读独立的时间列),或让上一轮过滤不依赖可被刷新的时间;至少补一条「同 head 重跑 + 文件缺失重下」的单测钉住期望的 changed 值。
R11 意见处理(
23af25bf)第 11 轮 1×P2 Act on。意见轨迹逐轮递减(R8:3 → R9:2 → R10:2 → R11:1),按再审计证伪条件维持有限清单。
P2 保留期时钟与「本轮新行」判定冲突:采纳「拆开两个时钟」——
review_assets.renewed_at保留期时钟列(AutoMigrate 补列;启动回填renewed_at = created_at,带列存在守卫兼容遗留库测试路径)。created_at恢复不可变的首次落账语义。renewed_at;退役 SELECT/DELETE 截止改COALESCE(renewed_at, created_at) < ?,配表达式索引idx_review_assets_retire_clock(EXPLAIN QUERY PLAN 验证命中 SEARCH),原 created_at 单列索引 DROP。created_at判「本轮新行」,该列不再被刷新后判定自然恢复正确——重下成功的旧行留在上一轮集合里,changed不再误报。测试钉住:重下成功断言
renewed_at刷新且created_at等于首次落账时刻(防回退);探测修复同规则;超龄测试拨钟改renewed_at。server 全量
go test -count=1 ./...+ gofmt + vet 通过。推送自动触发下一轮审查。R12 的 renewed_at 拆分本身未发现会直接造成线上故障的缺陷,但资产复核与 changed 判定仍有两条边界路径会丢历史截图或误报未变化。影响:uploads 卷瞬时 ENOENT 加一次重下失败即可永久回收既有证据;同 head 重跑且探测与建运行跨同步时,新增截图会被判成「未变化」。
@ -488,0 +490,4 @@// 命中行不复核的话一次竞态就是该端点永久 404——缺失按失败行重下,// 缓存命中或重新落盘都能自愈。只有 ErrNotExist 触发重下:其它 Stat// 错误(瞬时 I/O)保持原行,避免把有效路径误清。if seenBefore && existing.StoragePath != "" && !assetFileMissing(existing.StoragePath) {P2:问题:命中行只要 os.Stat 返回 ENOENT 就进入重下,重下失败(缓存目录本身不可用、网络失败)时 518-521 行把 storage_path 覆盖成空串;而 ENOENT 也可能来自 uploads 卷/子目录瞬时不可用或未挂载。影响:行一失去文件引用,该文件在卷恢复后就变成「无引用 + mtime 超宽限」的孤儿,下一轮 6h 的 SweepUnreferencedEntries 会把它永久删除,与本 PR「审查截图不可随意丢弃」的目标相反,renewed_at 只保护行、保护不到字节。建议:ENOENT 时先确认缓存目录可访问(Stat CacheDir 或父目录)再判缺失;或在重下成功前保留原 storage_path、只更新 error(失败行照常重试),不要用一次失败的重下清掉既有字节的唯一引用。
R12 意见处理(
b8f09a6d)第 12 轮 2×P2 + 1×Note 全部 Act on。意见轨迹 3→2→2→1→3——本轮两条 P2 都在 R11/R12 新语义与既有消费面的交界上(重下失败路径、探测跨同步时序),非同一机制反复生反例,按再审计证伪条件维持有限清单。
server 全量
go test -count=1 ./...+ gofmt + vet 通过。推送自动触发下一轮审查。本轮无 P0/P1:R12 两条修复(重下失败保留引用、prevRefs 截止改上一轮完成时刻)实现与测试一致,只找到两处边界问题。轮次可重叠时新截止会把本轮新行算进上一轮(changed 误报 false),重下失败保留路径后 error 与缺失文件对外都无法归因(500)。
@ -506,0 +520,4 @@// 好打在本 PR「审查截图不可随意丢弃」的目标上。命中行重下失败只// 刷 error,路径与元数据原样保留,下次复核再试;失败行本就无路// 径,维持空串。重下成功才是全新落账(含续期)。updates := map[string]any{"error": errText}P2:问题:重下失败只刷 error、保留 storage_path 之后,行会同时带非空路径与 error;但消费面只在 StoragePath=="" 时才把 Error 当作可归因原因下发(本文件 971 行),截图端点则直接 os.Open(server/internal/handler/review_agent.go:73),文件不在时经 HandleAppError 回 500「服务器内部错误」。影响:uploads 卷瞬时不可用(正是这条修复要保护的场景)期间,审查员拿到的是必然 500 的图片 URL 且看不到已记录的失败原因,无法判成 unmeasurable;卷恢复前这段窗口对外表现为服务端错误而非「截图不可用」。建议:AgentScreenshots 对非空路径再做一次存在性判断(或让 AgentScreenshotImage 在文件缺失时返回 404 并带出 asset.Error),缺失时按带原因的错误项下发;补一条「路径保留 + 文件缺失 + 重下失败」的 AgentScreenshots 用例钉住。
@ -789,0 +927,4 @@// changed 误报 false。上一轮完成之后写的行都属本轮证据;上一轮没有// 完成时刻(生产不该出现)时退回旧口径 run.CreatedAt,宁误报变化。prevCutoff := run.CreatedAtif prevRuns[0].FinishedAt != nil {P2:问题:prevCutoff 直接取 prevRuns[0].FinishedAt,没有与 run.CreatedAt 取更早者;manual rerun / 评论唤醒允许在同一 PR 上开新轮(openRoundTx 对 manual 恒新轮,不看在途运行,每用户 5 个在途),所以新轮的 run.CreatedAt 可能早于上一轮某个组合运行的 FinishedAt。影响:新轮在上一轮完成前写入的资产行(CreatedAt 落在 run.CreatedAt 与 prevFinished 之间)会被当成上一轮证据计入 prevRefs,若恰好是新增截图,两侧指纹相等、changed 误报 false,视觉审查被告知截图未变化——正是 R11/R12 想封口的反向误判,也违反注释里「宁误报变化」的口径。建议:截止取 min(prevRuns[0].FinishedAt, run.CreatedAt)(或先取上一轮所有运行的最晚 finished_at 再与 run.CreatedAt 取小),并补一条「上一轮仍在跑时开新轮、新行早于上一轮完成」的 changed 单测。
R13 意见处理(
33414b40)第 13 轮 2×P2 全部 Act on(两条都是 R12 修复的边界后果,非新故障模式类,维持有限清单):
server 全量
go test -count=1 ./...+ gofmt + vet 通过。推送自动触发下一轮审查。本轮新增 1 个 P2 与 1 个 Note:重叠轮次下探测行仍会被算进上一轮,changed 可能误报未变化;保留策略的非终态状态清单是硬编码且注释声称不会漂移。其余 R13 两条意见已修复,未发现新故障模式。
@ -789,0 +929,4 @@// 完成前且本轮创建前写的行按上一轮证据保留,其余都属本轮,宁误报// 变化不误报未变化。上一轮没有完成时刻(生产不该出现)时退回本轮// 创建时刻。prevCutoff := run.CreatedAtP2:问题:探测落账(recordProbeAssets)在 createVisualRuns 之前写行,探测行 CreatedAt 早于本轮视觉运行的 CreatedAt;本轮 prevCutoff 取 min(上一轮 FinishedAt, run.CreatedAt),而上一轮视觉运行仍在跑时 FinishedAt 为 NULL(这是 manual rerun/评论唤醒重叠轮次的正常状态,注释「生产不该出现」不成立),直接退回 run.CreatedAt。于是探测行 CreatedAt <= cutoff 会被算进 prevRefs。另外 prevRuns 查询只有 Order("round_number DESC").Limit(1),同一轮多个视觉运行里可能取到仍在跑的那个(FinishedAt=NULL),进一步放大概率。影响:同 head 手工重跑/评论唤醒时若 PR 描述新增截图,且新轮取截图时上一轮仍在途,新增截图的探测行被当作上一轮证据,两侧指纹相等 → changed=false,审查员被告知截图未变化,正是 R11/R12 想封口的反向误判。建议:FinishedAt 为空时不要退回 run.CreatedAt,改用不晚于上一轮运行 CreatedAt 的截止(或直接判 changed=true)以保持「宁误报变化」;同轮多运行时优先取已完成的运行;补「上一轮 status=running、探测行早于本轮视觉运行创建」的 changed 用例(现有重叠测试把新行放在 curCreatedAt 之后,未覆盖探测先行的真实时序)。
R14 意见处理(
27ca048b)第 14 轮 1×P2 + 1×Note 全部 Act on(意见轨迹 3→2→2→1→3→2;两条都还是重叠轮次时序与清单口径的收尾,维持有限清单):
finished_at IS NOT NULL,只认已完成的视觉运行作参照:在途轮次没有可沿用的结论,误当参照会把本轮探测行算进上一轮(正是 changed 误报 false 的通道)。没有已完成参照时 prevRefs 为空、changed 按默认 true(宁误报变化)。完成时刻因此恒非空,nil 回退分支整体删除;同轮多运行取到在途运行的概率随过滤一并消除。补「上一轮 status=running、探测行早于本轮视觉运行创建」的 changed 用例(三轮次场景:completed r1 / running r2 / 本轮 r3,探测行落 r2 窗口内,断言 changed=true)。model.ReviewRunActiveStatuses切片(挂在状态常量旁),退役 SELECT/DELETE 的守卫清单从切片生成;注释改为如实口径(切片是唯一事实源,新增在途状态须同步切片,漏加会在途运行连行带文件被退役)。新增全集对账单测:切片混入终态、或全集中任一在途状态未列,都会失败。server 全量
go test -count=1 ./...+ gofmt + vet 通过。推送自动触发下一轮审查。本轮未发现 P0/P1:上一轮运行只要 finished_at 非空就会成为 changed 的基准,失败/取消、从未取过截图的运行也不例外,仍有一个 P2;另有一处回收时点的注释/文档口径偏差。
@ -766,3 +905,3 @@changed := truevar prevRuns []model.ReviewRunif err := s.db.Where("project_id = ? AND repo_full_name = ? AND pr_number = ? AND integration_id = ? AND kind = ? AND round_number < ?",if err := s.db.Where("project_id = ? AND repo_full_name = ? AND pr_number = ? AND integration_id = ? AND kind = ? AND round_number < ? AND finished_at IS NOT NULL",P2:问题:prevRuns 只过滤 finished_at IS NOT NULL,failed/timed_out/cancelled 的运行(同样没有可沿用的视觉结论)也会成为 changed 参照;本 PR 新增的探测落账在视觉运行创建前就写行,一个准备阶段失败、从未调用 AgentScreenshots 的上一轮仍留下完整资产行。影响:同 head 下一轮(manual rerun/评论唤醒/重试)若截图集合不变,prevRefs 与 curRefs 相等 → changed=false,审查员被告知截图相对上一轮未变化,但上一轮实际无人看过这些截图。建议:参照查询加 status = completed(或至少排除 failed/timed_out/cancelled),并补「上一轮准备失败、本轮同集合」的 changed 用例。
R15 意见处理(
dc1b3a9f)第 15 轮 1×P2 + 1×Note 全部 Act on(意见轨迹 3→2→2→1→3→2→1 持续收敛,维持有限清单):
server 全量
go test -count=1 ./...+ gofmt + vet 通过。推送自动触发下一轮审查。R16 意见处理(
a264c191)第 16 轮 0×P0–P2,唯一 Note(注释口径漏网)Act on:
reviewAssetRetention常量注释与sweepAssetRetention内的快照注释两处旧口径(「下一轮扫描回收/留给下一轮」)已一并同步为「退役时 Chtimes 续期,宽限 24h 从退役时刻起算,约 24-30h 后的下一次扫描回收」,与 R15 已改的头注释及 docs/internal/reviews.md 一致。已全文件扫描确认无第三处旧口径。纯注释改动。server 全量
go test -count=1 ./...+ gofmt + vet 通过。推送自动触发下一轮审查。R17 意见处理(
38262fb6)第 17 轮 0×P0–P2,唯一 Note(快照注释语义写反)Act on:实际代码在退役之前 Pluck 出 referenced 集合,含即将退役行的 storage_path——本轮这些路径仍判为有引用、不回收;下一轮行已删、引用消失,宽限从退役时刻的 Chtimes 续期起算(24h)后收走。R16 改写时我把它写成了「释放的字节不在快照里,按无引用回收」,恰好相反,且暗示 Chtimes 续期是冗余——这正是审查指出的打穿保护的风险方向。已改为如实口径并核对实际代码(referenced 快照确实先于退役、含即将退役行)。
纯注释改动。server 全量
go test -count=1 ./...通过(quota kick 单测偶发超时与本改动无关,单测重跑 5/5 绿)。推送自动触发下一轮审查。