重构会话持久化:改为 SQLite 增量存储并移除 JSON 快照写入#343
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
/review |
|
/simplify |
|
|
||
| // ensureDB 懒加载数据库并执行 schema 初始化。 | ||
| func (s *SQLiteStore) ensureDB(ctx context.Context) (*sql.DB, error) { | ||
| s.once.Do(func() { |
There was a problem hiding this comment.
ensureDB 把调用方的 request-scoped ctx 直接传进 sync.Once 初始化链路。这样如果第一次调用刚好因超时/取消失败(例如 context canceled),initErr 会被永久缓存,后续即使用健康 context 也无法重试初始化,整个 store 会进入不可恢复状态。建议将一次性初始化与请求生命周期解耦(例如使用 context.Background() + 内部超时),或改为可重试的初始化门闩而不是 sync.Once 硬缓存失败。
|
|
||
| // ensureDB 懒加载数据库并执行 schema 初始化。 | ||
| func (s *SQLiteStore) ensureDB(ctx context.Context) (*sql.DB, error) { | ||
| s.once.Do(func() { |
There was a problem hiding this comment.
[high] ensureDB binds initialization to the first caller's ctx via sync.Once. If that first call is canceled/times out (or hits a transient init error), s.initErr is permanently cached and all future store operations fail until process restart. This makes initialization non-recoverable under transient failures. Consider decoupling one-time schema init from request-scoped context (or allowing retry when init fails).
|
|
||
| // ensurePathWithinBase 校验目标路径位于指定基目录内,避免路径越界。 | ||
| func ensurePathWithinBase(baseDir string, target string) error { | ||
| baseAbs, err := filepath.Abs(baseDir) |
There was a problem hiding this comment.
[medium][security] ensurePathWithinBase uses filepath.Abs + Rel, which is purely lexical and does not resolve symlinks. A symlink inside the project/assets tree can still redirect writes/reads outside the workspace while passing this check. For boundary enforcement, resolve trusted base/target with EvalSymlinks (or open with no-follow semantics) before containment checks.
ResultImplemented the unresolved review fixes and pushed to the PR branch What I changed
Validation
Notes
Commit
|
|
/code 补充测试率 |
…checks - replace sync.Once init cache with mutex-guarded retryable init path - resolve symlinks in containment checks to prevent asset path escapes - add regression tests for init retry and symlink escape cases Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
处理结果已按“补充测试率”完成补测并推送到当前 PR 分支。 变更内容
验证结果
提交信息
|
|
/code 处理冲突 |
Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
冲突处理结果已完成本次“处理冲突”。本次冲突点集中在依赖清单,已在当前 PR 分支完成整合并推送。 已完成内容
未做内容
|
Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
|
/code 我叫你解决合并冲突!! |
冲突处理完成已完成这次“解决合并冲突”,并把结果推送到分支 本次处理内容
验证
推送与 PR
|
Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
fix: resolve merge conflicts with main
|
@xgopilot 检查一遍当前修改有没有破坏项目中的图片输入链路 |
检查结论未发现本次 SQLite 持久化改造破坏图片输入主链路的问题。 核查范围
验证结果已执行并通过以下相关测试:
备注
|
|
@xgopilot 针对本次PR,请给我一份给AIreview用的提示词,针对的是修改中所有可能存在的风险点 |
AI Review 提示词(风险点全覆盖版)说明按本次触发指令,仅提供了“AI review 提示词”。 |
结论会有压力,而且在你这个会话场景里通常不划算把图片二进制直接塞进 SQLite 主库。你们这次 PR 采用的“附件落盘 + SQLite 存元数据”是更稳妥的默认方案。 对比(结合当前 PR)
建议
本次完成情况
|
|
@xgopilot 回答我一个问题:当前如果直接把图片数据存到sqlite中,会不会对数据库本身造成压力?如果改成图片之类的附件存到本地目录,sqlite只需保存图片路径,和原来的方案相比有什么优劣? |
结论会有压力。把图片二进制直接写进 SQLite(BLOB)在图片数量/体积上来后,会明显放大 DB 文件、WAL 文件和 checkpoint 成本;读写高峰时也更容易让数据库承担混合负载压力。 你说的“附件落本地目录,SQLite 只存元数据+路径”就是当前这个 PR 采用的方向( 两种方案对比
对本 PR 的建议
本次执行说明
|
|
@xgopilot 你是资深 Go 代码审查工程师。请对 PR #343(“重构会话持久化:改为 SQLite 增量存储并移除 JSON 快照写入”)做“风险导向”审查。 【审查目标】 【对比基线】
【项目约束(必须检查)】
【必查风险清单】
【输出格式(严格)】
【额外要求】
|
Findings
残余风险与盲区
|
背景
当前会话持久化基于单个
session.json快照文件:这次改动将会话持久化主路径重构为 SQLite 增量存储,不再兼容旧 JSON 会话数据。
主要改动
1. 会话存储从 JSON 改为 SQLite
internal/session新增SQLiteStoresession.dbforeign_keys=ON、busy_timeout=5000user_version管理 schema 版本2. 拆分会话头与消息正文
sessions表保存标题、更新时间、provider/model、workdir、task_state、todos、skills、token 统计等摘要信息messages表按session_id + seq逐条存储消息session_assets表保存资产元数据,二进制内容仍落盘到assets/3. Store 接口改为意图型写入
移除快照式整份
Save,改为:CreateSessionLoadSessionListSummariesAppendMessagesUpdateSessionStateReplaceTranscript这样 runtime 不再提交整份
Session快照,而是表达具体变更意图。4. runtime 持久化链路改为增量写入
CreateSessionAppendMessagesAppendMessages,并同步更新 provider/model/tokenAppendMessagesUpdateSessionStateReplaceTranscript5. 移除旧 JSON 主路径
session.json作为生产持久化格式6. 文档与测试同步更新
internal/session、internal/runtime、internal/app相关测试验证
已通过:
go build ./...go test ./internal/session ./internal/runtime ./internal/app当前覆盖率:
internal/session:86.5%internal/runtime:95.6%internal/app:92.5%说明:
go test ./...仍存在仓库内其他既有失败,主要不在本次会话持久化改造范围内影响
runtime.LoadSession/ TUI 仍保持完整加载单会话,不做懒加载